At the begining of the admin/index.php file in xoopsfaq module these two paragraphs can be found:

if (isset($HTTP_GET_VARS)) {
foreach ($HTTP_GET_VARS as $k => $v) {
$$k = $v;
}
}

if (isset($HTTP_POST_VARS)) {
foreach ($HTTP_POST_VARS as $k => $v) {
$$k = $v;
}
}

What are they for? To avoid SQL Injection? To avoid single quoting problems?

Thanks in advance.

P.S. In admin/index.php file of xoopspoll module only HTTP_POST_VARS are treated like this.

the loops are for assigning values to variables if the server is not using Register Globals.

Actually they could make it possible to do SQL injection more than it makes it possible to avoid it, depends on the quality of the following code.

Pages, which don't use GET variables shouldn't have the loop to prevent overwriting previously defined constants.

Personally, I use the POST loop and then define each variable, I want through GET, in individual lines.

Ok, I see.

By the way, which should be the "best" or recommended way to avoid SQL injection? Does XOOPS do any sort of treatment for that purpose, so that you don't have to take care of it in your module?

Could this code be a good solution?

foreach ($HTTP_POST_VARS as $varname => $value) {
if (is_string($value)) $value = addslashes($value);
$vars[$varname] = $value;
}

And then use the values from $vars to store them in the database and always do a stripslashes before showing any string retrieved from the database.

Thanks for your response.

whether you use ${$k} or $vars[$varname] shouldn't change much (AFAIK but I'm definitely no security expert)

XOOPS does something for the security in the way that it is impossible to do anything but SELECT queries if your variables were given via the GET method, but you should always be aware of it in your module, too - especially with free-text forms.

addslashes might be a good way of preventing SQL injection - but again, I am no expert.

isn't really a good idea, is it? It's manually "registering" all form input as variables, and can result in surprises if you neglect to otherwise initialize a variable.

A safer approach is to explicitly initialize all variables that are actually used:

$var1 = $HTTP_POST_VARS['var1'];
$var2 = $HTTP_POST_VARS['var2'];
...

I have to agree with Dave_L on this one. I think those lines were added to XOOPS code in a hurry at some point to fix complaints about not working with register_globals off.

It is true if you use the "loop" method, if you forget to initialize a variable in the module, then you can potentially be open to a variety of attacks depending on how that variable is used. i.e. The attacker can feed a value into that variable by adding an argument to the URL.

As a *preferred* method, I would choose Dave_L's approach, or simply use the full $HTTP_POST_VARS['var1'] in the places where you actually need it.

So far this really doesn't relate specifically to SQL injection. The idea of doing 'addslashes' on all non-strings is not good as it leads to unpredictable state of your variables. i.e. You should only 'addslashes' if you really need to, and only the rest of your module code will really know this. A loop at the beginning of your code doesn't know this. Also, 'addslashes' is not really a good way to make things database-safe -- it works for MySQL but not for many other databases which may be supported in the future.

The way to avoid SQL injection is to not use HTTP_POST_VARS or HTTP_GET_VARS etc. **directly** in SQL statements.

Use quoteString:

$xoopsDB->query('SELECT * FROM sometable WHERE user=' . $xoopsDB->quoteString($HTTP_GET_VARS['username']));

but DO NOT use:

$xoopsDB->query('SELECT * FROM sometable WHERE user=' . $HTTP_GET_VARS['username']);

Anyways, there are other issues related to HTTP_VARS and SQL injection is just one of them. The other big thing to worry about is if you directly echo an $HTTP_VAR to the page you are creating. If someone put malicious HTML or javascript into that value, then that malicous code appears on your webpages where it could do anything from disrupting the page layout to stealing a user's cookies (which can allow password stealing and session hijacking). I only know a little about this stuff but the 'net has a huge amount of info if you're interested.

EDIT: by the way, the usual fix for making things display-safe is to use htmlspecialchars($HTTP_GET_VARS['somevar']), but depending exactly what the variable is used for this may or may not work.

Thanks a lot to all of you. I got the necessary information.