Advertisement:

Author Topic: Security practices on creating modifications  (Read 46988 times)

Offline Harzem

  • SMF Hero
  • ******
  • Posts: 5,384
  • Gender: Male
  • I know, my avatar is nerve-wracking!
Security practices on creating modifications
« on: October 11, 2006, 05:25:18 PM »
Understanding SMF Security

SMF is proud of being one of the most secure forum systems in the world. Of course this was not done magically, SMF has strict rules and practices to achieve security. Here is some guidelines to understand the main and most important techniques for SMF-level security.

Permissions

SMF relies on a powerful permission system. As a mod writer, you should already know how you can create and check for permissions. Permissions contribute the security by providing some functions to some member(group)s, and not to others. You should be careful when you are checking permissions.

- Permissions are to be used before displaying the action page. Before displaying a page that needs some permissions, first check the permission and then display the page.
- Permissions are also to be checked before the action is taken. When your mod displayed a page, the user enters some values to the form or clicks some links. When the user completes his action, before transferring the data to database (or processing any other action), you MUST check the permissions again. This is more important than checking the permission before displaying the page.


Session Checks

Session checking is one of the two most important practice to make SMF secure. It is for preventing malicious users to make other users do something that they originally didn't intend to.

What is session checking?

There are two types of links in SMF. One is to "open a window" to do something, the other is to "do something" directly.

For example, clicking on the "move" button of a topic opens a new window before doing anything. It asks you the board you want to move and the redirection topic. When you click on a "move topic" link, nothing happens but it displays a new screen. No actual movement is processed.

But clicking "lock" button for a topic is different. It directly locks the topic. The admin is not displayed a new confirmation screen, but the topic is locked directly.

This second case is what you should be careful about. For example, you have created a modification, which is used to increase other users' karma points by 10. You used this link as the karma increase link:

forum.com/index.php?action=karma;sa=increase;u=156

This link is supposed to increase the karma of user ID 156, and it works like a charm. When a user clicks the link, user 156 directly gains 10 more karma. But this link is insecure!

Because, it is a direct link. A malicious user can exploit it to increase his karma by thousands! He can do two things:

- Put a fake link: www.google.com , and any user clicking on it will have accidentally increased 156's karma.

- Request a php image in the post: [img]forum.com/index.php?action=karma;sa=increase;u=156[/img] , which will be displayed to the users each time they visit the page. And this image code will request the target page each time, increasing 156's karma rapidly.


So, in order to prevent fooling of an admin/user by an external call of such a link, a "sesc=<PHPSESID>" is appended after these links. Like:
forum.com/index.php?action=karma;sa=increase;u=156;sesc=d113eb64c27777ca1037d8c0ff51ae92

In this case, everyone has a unique link to increase somebody's karma. This sesc is different for every member, also it changes everytime the users log in. So inserting a fake hyperlink won't work: www.google.com. This link only works is a user with session ID d113eb64c27777ca1037d8c0ff51ae92 clicks on the link.

To note: there is no way someone can "guess" other users' session IDs. So, your link is secured. (Of course you should also learn how to "check" for session ids.)

When do you need session checking?

As said, you should use session checking before doing a direct action.
If the link opens a new window before actually doing something, there is no need for session checking.

Here are some examples:
(These links open a new window before doing something)
- Move topic
- Merge topics
- Delete user (in profile view)
- Post & Reply

(These links DO their job without opening a window)
- Lock topic
- Remove topic
- Sticky topic
- Mark As Read
- Notify

You should try and see that these links really have that session IDs (or don't have).

Verifying the sessions will also be described below.


What is form sessions?

There is a second type session checking in SMF. This is applied when someone inputs and posts a form, like a post form or a profile editing form.

When a user posts a form, you should be sure that the form is actually taken from the correct page. A form can easily be fooled by a malicious user.

Say, you are a malicious user. You want to become an admin. You know that the actual admin needs to change your membergroup from your profile, and then hit "save". When he hits "save", a form data is submitted to SMF.

You create a webpage on another website. It includes hidden form elements, including a new membergroup data. And the page also has a form button.

Then you convince the admin to click on it in some way. Like "hey, look at http://mycoolsite.com". When the admin goes to the page, he sees two buttons: "I'm over 18" and "I'm under 18". He clicks either one of them. And, a form is submitted!

The form target was actually the SMF site itself. When the admin clicked the button, he sent a form to his own forum accidentally, which was designed to make you an admin!

ALL forms must include session checks in it! There are NO forms that don't need session verification!

How do I secure the forms?

Simple: form sessions. It is line URL sessions. You put a session ID value to the form.
<input type="hidden" name="sc" value="d113eb64c27777ca1037d8c0ff51ae92" />
When the form is submitted by the user, SMF will check whether a correct value of session was recieved or not. If not recieved, SMF won't do that function.

Since the malicious user can't guess the correct value of the session ID of the admin, he can't create a fake form.

How do I apply session checking into my code?

* Applying URL sessions

When designing templates or links to be displayed to the users, you should append "sesc=<PHPSESSID>" to the links. This is achieved by this simple code:
sesc=' . $context['session_id'] . '

Example:



echo '
<a href="' 
,
$scripturl '?action=karma;sa=increase;u=156;sesc=' $context['session_id'] , '">' $txt['increase'] , '</a>';



or


// (don't forget to define the global $sc)

echo '
<a href="' 
,
$scripturl '?action=karma;sa=increase;u=156;sesc=' $sc '">' $txt['increase'] , '</a>';



Make sure you verify that the URL really has the session ID in it now.

* Applying form sessions

You should be sure that the form contains a hidden element between <form> and </form> tags. It is usually inserted right before or after the "save" button.
This element is <input type="hidden" name="sc" value="d113eb64c27777ca1037d8c0ff51ae92" />, which can be inserted like this:



echo '
<input type="hidden" name="sc" value="'
$context['session_id'], '" />';



or


// (don't forget to define the global $sc)

echo '
<input type="hidden" name="sc" value="'
$sc'" />';



Make sure you verify that the form has the hidden element in it. If you have multiple forms in your page, all forms should have their sc elements.

* Checking URL sessions

Now the important part, checking the sessions. The URL has a sesc, but it is true?

This is fairly an easy process to check. You simply add a single line of code before actually doing the job.
checkSession('get');

An example:



// Assuming that you have already verified that $userID is valid,
function IncreaseKarmaBy10($userID)
{
	
global 
$db_prefix;
	

	
isAllowedTo('increase_karma'); // Always do the permission check!
	

	
checkSession('get'); // Verify that the URL has a "sesc" variable in it.
	

	
// Do the actual job
	
db_query("
	
	
UPDATE 
{$db_prefix}members
	
	
SET karmaGood = karmaGood + 10
	
	
WHERE ID_MEMBER = 
$userID
	
"
__FILE____LINE__);
}



checkSession('get'); function will

- check whether the URL has a "sesc" in it,
- do nothing if the URL has a correct sesc,
- throw a "session verification failed" error if the URL:
   - doesn't have a sesc,
   - has an incorrect sesc.

This is how you can practice session verification on an URL.

* Checking form sessions

Verifying that a form has a correct session is as simple as the URL case. You use
checkSession();
to see that the form data has the hidden element "sc" in it and it is correct.

Example:



// Assuming that you have already verified that $userID and $groupID is valid,
function ChangeMemberGroup($userID$groupID)
{
	
global 
$db_prefix;
	

	
isAllowedTo('manage_members'); // Always do the permission check!
	

	
checkSession(); // Verify that the form has a "sc" element in it.
	

	
// Do the actual job
	
db_query("
	
	
UPDATE 
{$db_prefix}members
	
	
SET ID_GROUP = 
$groupID
	
	
WHERE ID_MEMBER = 
$userID
	
"
__FILE____LINE__);
}



checkSession(); function will

- check whether the form has a "sc" element in it,
- do nothing if the sc is correct,
- throw a "session verification failed" error if the form:
   - doesn't have a sc,
   - has an incorrect sc.


After you used these session verification systems, your forms and links are more secure. Don't forget that:

- ALL directs link to do a job should have "sesc" in it. The links that opens a new window before actually doing a job won't need "sesc" in it.
- ALL form should have "sc" elements in it.
- A session ID of a user can't be guessed by another user.



Input Validations

Input validation is the other very important security practice that a mod MUST use.

What is input validation?

Input validation is to check whether a data sent to SMF is actually the data that was expected. For example, you made a mod to increase the karma of a user by 10. The increasing URL has the user ID.
forum.com/index.php?action=karma;sa=increase;u=156;sesc=d113eb64c27777ca1037d8c0ff51ae92

You send the user ID to the database by


	
$userID $_REQUEST['u'];
	
db_query("
	
	
UPDATE 
{$db_prefix}members
	
	
SET karmaGood = karmaGood + 10
	
	
WHERE ID_MEMBER = 
$userID
	
"
__FILE____LINE__);


But is $_REQUEST['u'] safe? Consider this:

forum.com/index.php?action=karma;sa=increase;u=156+OR+1;sesc=d113eb64c27777ca1037d8c0ff51ae92

u=156+OR+1 will make $_REQUEST['u'] = "156 OR 1". When this is sent to database query, this will be the query:


	
"
	
	
UPDATE 
{$db_prefix}members
	
	
SET karmaGood = karmaGood + 10
	
	
WHERE ID_MEMBER = 156 OR 1
	
"


"OR 1" will result in an increase of everyone's karma, not only the user 156. This might not look that malicious, but there are cases it is.

Consider this:

You have a mod, a profile field that is stored in members table. It is asking you the name of your favorite game. You are sending the data using a form within your profile. And your form sends $_POST['favorite_game'] as the input.

This is your code:


	
db_query("
	
	
UPDATE 
{$db_prefix}members
	
	
SET favoriteGame = '
$_POST[favorite_game]'
	
	
WHERE ID_MEMBER = 
$ID_MEMBER
	
"
__FILE____LINE__);


How can this be exploited? It will be exploited by sending a malicious input as:
CounterStrike', ID_GROUP = '1

Now put this into the query:


	
"
	
	
UPDATE 
{$db_prefix}members
	
	
SET favoriteGame = 'CounterStrike', ID_GROUP = '1'
	
	
WHERE ID_MEMBER = 156
	
"


And the user now has an ID_GROUP of 1, which means he is now an admin.


In the first case, the input "u" was not an integer. In the second case, "favorite_game" was not properly inserted into the query.


You should be sure that your variables are validated before being used:

- Integers should be integers.
$_REQUEST['u'] = (int) $_REQUEST['u'];

- String should be "escaped". Not all of the below, but when necessary:

$_POST['favorite_game'] = htmlspecialchars($_POST['favorite_game']);
$_POST['favorite_game'] = addslashes($_POST['favorite_game']);
$_POST['favorite_game'] = stripslashes($_POST['favorite_game']);
$_POST['favorite_game'] = un_htmlspecialchars($_POST['favorite_game']);

The last one is defined by SMF itself. It is similar to htmlspecialchars_decode of php. Look for the function definitions of these at php.net to get more information.

Always have a look at SMF core codes itself, to decide where you must use validation and how. For example, see Profile.php and track the use of validation functions used on a profile field variable until it is inserted to the database.

Of course there are more security measures that you must take when creating mods, but these are the fundamental ones widely used by SMF itself.
« Last Edit: October 05, 2008, 02:23:31 PM by H »

Offline Daniel15

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 4,221
  • Gender: Male
  • http://dan.cx/
    • daaniel on Facebook
    • Daniel15 on GitHub
    • daniel15 on LinkedIn
    • @Daniel15 on Twitter
    • Daniel15
Re: Security practices on creating modifications
« Reply #1 on: October 27, 2006, 06:16:41 AM »
Nice post, I'll be sure to remember some of these techniques ;)

Quote
How can this be exploited? It will be exploited by sending a malicious input as:
CounterStrike', ID_GROUP = '1
I thought that SMF escapes quotes in all GET and POST variables automatically?
Daniel15, former Customisation team member, resigned due to lack of time. I still love everyone here :D.
Go to smfshop.com for SMFshop support, do NOT email or PM me!

Offline Harzem

  • SMF Hero
  • ******
  • Posts: 5,384
  • Gender: Male
  • I know, my avatar is nerve-wracking!
Re: Security practices on creating modifications
« Reply #2 on: October 27, 2006, 01:21:00 PM »
Nice post, I'll be sure to remember some of these techniques ;)

Quote
How can this be exploited? It will be exploited by sending a malicious input as:
CounterStrike', ID_GROUP = '1
I thought that SMF escapes quotes in all GET and POST variables automatically?

It was just to make people think about unexpected issues. There are still ways to play with inputs if the mod is not well designed.

Offline Aquire

  • Semi-Newbie
  • *
  • Posts: 28
Re: Security practices on creating modifications
« Reply #3 on: October 28, 2006, 04:34:40 PM »
superB post ;D

Offline miseryshining

  • Jr. Member
  • **
  • Posts: 173
Re: Security practices on creating modifications
« Reply #4 on: November 17, 2006, 10:15:37 AM »
thank you so much for this information, it really made clear that i have some work to do on the mod i'm currently developing.

I'd wish all useful information for developers would be bundled in a handbook of some sort, but that's a different topic. Something in the form of a wiki might not be a bad idea (even if only a documentation team would have access to most of the parts).

Offline Anguz

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 3,430
  • Gender: Male
    • cristianlavaque.com
Re: Security practices on creating modifications
« Reply #5 on: December 08, 2006, 12:54:51 PM »
Magnificent post, HarzeM. Thanks for helping teach others how to keep SMF as secure as it's been.
Cristián Lávaque http://cristianlavaque.com

frankeistein

  • Guest
Re: Security practices on creating modifications
« Reply #6 on: December 08, 2006, 03:02:07 PM »
thanks harzem ;)

Offline Sarke

  • Jr. Member
  • **
  • Posts: 315
Re: Security practices on creating modifications
« Reply #7 on: January 17, 2007, 06:00:07 AM »
Great post, thanks!

I think this topic should be a sticky (had I not searched for "mod security" I wouldn't have seen this); every MOD maker should read it.
My MODs          Please don't PM me for support, post in the appropriate topic.

Offline Minare

  • Sophist Member
  • *****
  • Posts: 1,233
  • Gender: Male
  • Yapayalnız Kalmak İskelelerde !
    • Meryemce
Re: Security practices on creating modifications
« Reply #8 on: March 12, 2007, 12:30:57 PM »
Great post, thanks!

I think this topic should be a sticky (had I not searched for "mod security" I wouldn't have seen this); every MOD maker should read it.

I think the same, such a useful topic should be sticky so that everyone be careful about it.

Thanks HaRZeM bro  ;)
O güzel Râsulu Yesrip yolunda
Hicret coşkusuyla aramalısın
Ayrılık Anne'den Baba'dan Yâr'dan
Ezelden geçerek kavuşmalısın

http://www.meryemce.biz

Offline matematik

  • Jr. Member
  • **
  • Posts: 247
  • Gender: Male
Re: Security practices on creating modifications
« Reply #9 on: May 09, 2007, 10:34:23 PM »
thank you so much my friend

Offline KGIII

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 6,293
  • Gender: Male
  • If you can build it, I can wreck it.
    • Web Hosting
Re: Security practices on creating modifications
« Reply #10 on: May 16, 2007, 06:11:10 PM »
Moved to tips and tricks and added to the index.
My PC Support Forum
Please ask in-thread before PMing
                   SMF Help
                   Visit My Blog

How can we improve the support process?:
http://www.simplemachines.org/community/index.php?topic=163533.0

SMF vs. Godzilla? Who do you think will win?

Offline Daniel15

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 4,221
  • Gender: Male
  • http://dan.cx/
    • daaniel on Facebook
    • Daniel15 on GitHub
    • daniel15 on LinkedIn
    • @Daniel15 on Twitter
    • Daniel15
Re: Security practices on creating modifications
« Reply #11 on: May 17, 2007, 05:46:16 AM »
Moved to tips and tricks and added to the index.
I'm hoping it doesn't sink down and get lost... :)
Daniel15, former Customisation team member, resigned due to lack of time. I still love everyone here :D.
Go to smfshop.com for SMFshop support, do NOT email or PM me!

Offline KGIII

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 6,293
  • Gender: Male
  • If you can build it, I can wreck it.
    • Web Hosting
Re: Security practices on creating modifications
« Reply #12 on: May 17, 2007, 07:45:08 AM »
I suppose we COULD sticky this one if we really wanted? No one would notice? I did add it to the index as well.
My PC Support Forum
Please ask in-thread before PMing
                   SMF Help
                   Visit My Blog

How can we improve the support process?:
http://www.simplemachines.org/community/index.php?topic=163533.0

SMF vs. Godzilla? Who do you think will win?

Offline Sarke

  • Jr. Member
  • **
  • Posts: 315
Re: Security practices on creating modifications
« Reply #13 on: May 17, 2007, 08:53:20 PM »
Personally I think it is better suited in SMF Coding Discussion than Tips and Trick becuase isn't Tips and Trick more about small tweaks to SMF etc?

Quote
The Tips and Tricks board contains a collection of manual SMF modifications.

This topic is more for reference as a guidline for mod authors.  It's seems more burried in a sub-board like this, and in a long index.  I vote for sticky it, and/or include it in groundup's SMF Coding Guidelines.
My MODs          Please don't PM me for support, post in the appropriate topic.

Offline Daniel15

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 4,221
  • Gender: Male
  • http://dan.cx/
    • daaniel on Facebook
    • Daniel15 on GitHub
    • daniel15 on LinkedIn
    • @Daniel15 on Twitter
    • Daniel15
Re: Security practices on creating modifications
« Reply #14 on: May 18, 2007, 06:10:09 AM »
Sarke, I agree completely - It should be in the Coding Discussion forum. And I agree about the sticky as well :D
Daniel15, former Customisation team member, resigned due to lack of time. I still love everyone here :D.
Go to smfshop.com for SMFshop support, do NOT email or PM me!

Offline karlbenson

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 15,629
  • Gender: Male
    • @mortonssols on Twitter
    • Criminal Solicitors
Re: Security practices on creating modifications
« Reply #15 on: May 23, 2007, 10:41:29 AM »
great post.  I'll keep it in mind when coding my mods

Offline KGIII

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 6,293
  • Gender: Male
  • If you can build it, I can wreck it.
    • Web Hosting
Re: Security practices on creating modifications
« Reply #16 on: May 25, 2007, 05:21:18 PM »
Well someone put it where they want. ;) I just went by the team boards suggestion to put this one here as well as the other one.
My PC Support Forum
Please ask in-thread before PMing
                   SMF Help
                   Visit My Blog

How can we improve the support process?:
http://www.simplemachines.org/community/index.php?topic=163533.0

SMF vs. Godzilla? Who do you think will win?

Offline SneakyWho_am_i

  • Semi-Newbie
  • *
  • Posts: 10
  • Gender: Male
    • sneakywhoami.com
Re: Security practices on creating modifications
« Reply #17 on: July 23, 2007, 04:50:23 PM »
This is a most excellent post/topic :)

Offline dcmouser

  • Charter Member
  • Jr. Member
  • *
  • Posts: 187
    • donationcoder.com
Re: Security practices on creating modifications
« Reply #18 on: August 10, 2007, 12:31:47 AM »
Fantastic post. Thank you.
proud member of donationcoder.com (forum)

Offline Daniel15

  • SMF Friend
  • SMF Hero
  • *
  • Posts: 4,221
  • Gender: Male
  • http://dan.cx/
    • daaniel on Facebook
    • Daniel15 on GitHub
    • daniel15 on LinkedIn
    • @Daniel15 on Twitter
    • Daniel15
Re: Security practices on creating modifications
« Reply #19 on: August 10, 2007, 04:24:21 AM »
I've added this to the SMFMods wiki at http://www.smfmods.org/wiki/Security_practices. If you like, please feel free to add to it :)
Daniel15, former Customisation team member, resigned due to lack of time. I still love everyone here :D.
Go to smfshop.com for SMFshop support, do NOT email or PM me!