Simple Machines Community Forum

Customizing SMF => Tips and Tricks => Topic started by: Harzem on October 11, 2006, 05:25:18 PM

Title: Security practices on creating modifications
Post by: Harzem 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 (http://www.simplemachines.org/community/index.php?topic=59366.0). 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 (http://forum.com/index.php?action=karma;sa=increase;u=156) , 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 (http://forum.com/index.php?action=karma;sa=increase;u=156;sesc=d113eb64c27777ca1037d8c0ff51ae92). 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 (#post_howsesc).)

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 (#post_howsc).


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.
Title: Re: Security practices on creating modifications
Post by: Daniel15 on October 27, 2006, 06:16:41 AM
Nice post, I'll be sure to remember some of these techniques ;)

QuoteHow 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?
Title: Re: Security practices on creating modifications
Post by: Harzem on October 27, 2006, 01:21:00 PM
Quote from: daniel15 on October 27, 2006, 06:16:41 AM
Nice post, I'll be sure to remember some of these techniques ;)

QuoteHow 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.
Title: Re: Security practices on creating modifications
Post by: Aquire on October 28, 2006, 04:34:40 PM
superB post ;D
Title: Re: Security practices on creating modifications
Post by: miseryshining 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).
Title: Re: Security practices on creating modifications
Post by: Anguz 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.
Title: Re: Security practices on creating modifications
Post by: frankeistein on December 08, 2006, 03:02:07 PM
thanks harzem ;)
Title: Re: Security practices on creating modifications
Post by: Sarke 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.
Title: Re: Security practices on creating modifications
Post by: Minare on March 12, 2007, 12:30:57 PM
Quote from: Sarke 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.

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

Thanks HaRZeM bro  ;)
Title: Re: Security practices on creating modifications
Post by: matematik on May 09, 2007, 10:34:23 PM
thank you so much my friend
Title: Re: Security practices on creating modifications
Post by: KGIII on May 16, 2007, 06:11:10 PM
Moved to tips and tricks and added to the index.
Title: Re: Security practices on creating modifications
Post by: Daniel15 on May 17, 2007, 05:46:16 AM
Quote from: KGIII on May 16, 2007, 06:11:10 PM
Moved to tips and tricks and added to the index.
I'm hoping it doesn't sink down and get lost... :)
Title: Re: Security practices on creating modifications
Post by: KGIII 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.
Title: Re: Security practices on creating modifications
Post by: Sarke 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?

QuoteThe 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 (http://www.simplemachines.org/community/index.php?topic=159824.msg1019681#msg1019681).
Title: Re: Security practices on creating modifications
Post by: Daniel15 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
Title: Re: Security practices on creating modifications
Post by: karlbenson on May 23, 2007, 10:41:29 AM
great post.  I'll keep it in mind when coding my mods
Title: Re: Security practices on creating modifications
Post by: KGIII 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.
Title: Re: Security practices on creating modifications
Post by: SneakyWho_am_i on July 23, 2007, 04:50:23 PM
This is a most excellent post/topic :)
Title: Re: Security practices on creating modifications
Post by: dcmouser on August 10, 2007, 12:31:47 AM
Fantastic post. Thank you.
Title: Re: Security practices on creating modifications
Post by: Daniel15 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 :)
Title: Re: Security practices on creating modifications
Post by: Neorics on September 22, 2007, 09:25:03 AM
so correct me if i'm wrong but this tip is only for mod writers? i mean does SMF have this feature already in it's system?

cause i'm planning to customize the 'edit profile' page to restrict certain fields to be able to be viewed/edited by certain member groups and i'll be using if($context statemets

so is it safe to do that?
Title: Re: Security practices on creating modifications
Post by: karlbenson on September 22, 2007, 05:27:06 PM
any fields you add where users can enter information need

-sanitising (eg casting as integer or removing bad characters)
-validating (eg if it was a youtube id, you would validate to check its a-zA-Z0-9- in a preg_match)
-escaping (if being inserted in the database) addslashes___resursive($data);
Title: Re: Security practices on creating modifications
Post by: Neorics on September 23, 2007, 03:11:16 AM
Quote from: karlbenson on September 22, 2007, 05:27:06 PM
any fields you add where users can enter information need

-sanitising (eg casting as integer or removing bad characters)
-validating (eg if it was a youtube id, you would validate to check its a-zA-Z0-9- in a preg_match)
-escaping (if being inserted in the database) addslashes___resursive($data);

ok... i'm totally lost... can you point me to a direction where i can read how to do those you mentioned?
Title: Re: Security practices on creating modifications
Post by: karlbenson on September 23, 2007, 08:30:51 AM
Well there isn't any specific smf code functions for doing the above.

You will need to use functions like preg_match, eregi, preg_replace with regex strings.

Santiizing eg if you've got an integer you can make sure its an integer
$string = (int) $string;

If it wasn't a number, it will be 0.

You'll find lots of examples of validating/santizing in mods.
Eg my youtube mod checks if the data being parsed is a valid youtube id (a-zA-Z0-9-)

if (preg_match(\'#^([0-9A-Za-z-_]{11})#i\', trim($data[0]), $matches)) {

More info
http://uk3.php.net/preg_match
http://uk3.php.net/preg_replace
Title: Re: Security practices on creating modifications
Post by: Neorics on September 23, 2007, 09:32:54 AM
thanks, omg i didnt realize that SMF have a google ad at the bottom... just saw it after opening it in IE...
Title: Re: Security practices on creating modifications
Post by: White Girl on March 09, 2008, 08:57:46 AM
Thank you for it :)
Title: Re: Security practices on creating modifications
Post by: BuЯЯЯЯaK on March 09, 2008, 06:05:01 PM
Great post thx harzem :)
Title: Re: Security practices on creating modifications
Post by: Informatics on March 11, 2008, 02:34:41 AM
wow... cool, thanx for posting.
Modders have to read this
Title: Re: Security practices on creating modifications
Post by: Kader on March 11, 2008, 05:52:54 AM
Hi all

Can somebody shed some light on how:
db_query()

can replace:

$link = mysql_connect('host', 'root', 'password')
    or die('Could not connect: ' . mysql_error());

This is to avoid having the password hanging around in a file.

Thanx all
Title: Re: Security practices on creating modifications
Post by: metallica48423 on March 14, 2008, 05:48:49 PM
have you looked at the function DB entry for db_query? 

db_query will use the SMF connection to make the query in the first parameter of it.  The second and third parameter should be __FILE__ and __LINE__

ie:


$id = db_query("
    SELECT ID_MEMBER
    FROM smf_members
    WHERE realName = 'metallica48423'", __FILE__, __LINE__);


will store the result of the query in $id

Title: Re: Security practices on creating modifications
Post by: Sami Ekici on May 26, 2008, 06:17:21 PM
teşekkürler Harzem Bey
Title: Re: Security practices on creating modifications
Post by: Sarge on July 05, 2008, 06:44:40 AM
Quote from: SA(^_^)Mi on May 26, 2008, 06:17:21 PM
teşekkürler Harzem Bey

SA(^_^)Mi, this is an English language board. The Turkish language board is located at: Türkçe (Turkish) (http://www.simplemachines.org/community/index.php?board=76.0)

But I understand that you are saying "Thank you, Mr. Harzem" :)
Title: Re: Security practices on creating modifications
Post by: Sami Ekici on July 29, 2008, 06:32:01 AM
Quote from: Sarge on July 05, 2008, 06:44:40 AM
Quote from: SA(^_^)Mi on May 26, 2008, 06:17:21 PM
teşekkürler Harzem Bey

SA(^_^)Mi, this is an English language board. The Turkish language board is located at: Türkçe (Turkish) (http://www.simplemachines.org/community/index.php?board=76.0)

But I understand that you are saying "Thank you, Mr. Harzem" :)

Thats Oky. I'm Sorry..