need clarification on coding standards

[Jakeman]

I have been working on my new site for several months now. I have been making extensive use of $_POST and $_REQUEST in some of my custom scripts.

I just came across the "code standards" section of the vB3 documentation - http://www.vbulletin.com/docs/html/codestandards_gpc


$_GET and $_POST variables should be run through the globalize() function in order to ensure that they have evil magic quotes removed from them before being used, with the exception of $_REQUEST['do'] and $_POST['do'], which is used as the controlling variable for deciding which branch of a script is executed. Do not use $_GET / $_POST / $_REQUEST etc. variables in templates.


1) What are magic quotes and why are they evil? :eek:

2) I have been using $_POST and $_REQUEST in the templates. Is this horribly wrong or is it just a harmless thing like some of the other coding standards?

[Dean C]

1/ When magic_quotes are on, all ' (single-quote), " (double quote), \ (backslash) and NUL's are escaped with a backslash automatically.

2/ There's nothing wrong with it but it's a little redundant. If you use the globalize function as other vB pages do then you can use the unglobalized variable version which in general just makes it cleaner code.

Good luck :)!

[Xenon]

as an addition to 2)

it's not recommended to use $_REQUEST or $_POST variables directly in templates, because they can be directly edited by the user (just a html change and you can have some bad things in ;))
it's not very problematical, as normally it couldn't hurt on templates, but just to be on the save side, it's better to always use the globalize() function and then use the globalized vars in templates.

[Jakeman]

oic

I validate all REQUEST and POST data very carefully, so I don't think I'm in any danger of injection.

Is the magic quotes thing the same as magic_quotes_gpc? I already use this code when I addslashes to POST data:


// IF M_Q_GPC IS NOT ENABLED, THEN PARSE FOR INJECTION
// M_Q_GPC AUTOMATICALLY PARSES ALL POSTED DATA IF ENABLED
if (!get_magic_quotes_gpc())
{
$_POST['var'] = addslashes($_POST['var']);
}


So I think I have that covered.

[Dean C]

Even the best coders can make slip ups which lead to injection :) The code you've posted above doesn't quite make sense to me. What i'd do is this:


if (!get_magic_quotes_gpc())
{
foreach($_POST AS $postkey => $postval)
{
if(is_string($postval))
{
$_POST["$postkey"] = addslashes($postval);
}
}
}


But doesn't vB3 do all this automatically :s? You can just use the globalize function anyway to do all what I posted above. And it helps clean up strings, arrays, files and integers too :)

[Jakeman]

I am handling the specific REQUEST and POST vars that I use rather than handling the whole arrays. When I add a REQUEST or POST var I add another line for it.

[Dean C]

So if you have 3 request vars you'd have this 3 times??:


// IF M_Q_GPC IS NOT ENABLED, THEN PARSE FOR INJECTION
// M_Q_GPC AUTOMATICALLY PARSES ALL POSTED DATA IF ENABLED
if (!get_magic_quotes_gpc())
{
$_POST['var'] = addslashes($_POST['var']);
}

[Jakeman]

// IF M_Q_GPC IS NOT ENABLED, THEN PARSE FOR INJECTION
// M_Q_GPC AUTOMATICALLY PARSES ALL POSTED DATA IF ENABLED
if (!get_magic_quotes_gpc())
{
$_POST['var1'] = addslashes($_POST['var1']);
$_POST['var2'] = addslashes($_POST['var2']);
$_POST['var3'] = addslashes($_POST['var3']);
}

[Dean C]

Ok... well with vB3 you might as well use their globalize function as it does checks on other things suchs as arrays, integers, files. As well as this it also deals with the magic quotes problem so it's standard on any vBulletin installation to use it :):


globalize($_POST, array('posthash' => STR_NOHTML, 'poststarttime' => INT, 'stickunstick' => INT, 'openclose' => INT));


Straight from editpost.php :)

The first arguement to the function is either $_POST or $_REQUEST then you pass an array of $_POST/$_REQUEST values you want to validate/strip etc. If you take a look at the function in (I think) functions.php you'll see what it can validate :)