Discussion (and consensus): get_magic_quotes_gpc / addslash

registered · Posted: Tue Aug 29, 2006 12:52 pm

This is reposted from previous thread, but I want to provide its own discussion thread as well as voting. I hope this will start building on the changes from the Patched files, into a basis for RavenNuke and any other phpNuke distributions.

------------------------

I want to deal with the magic_quotes issue first. It would be good to know what conditional all input variables come in, so we aren't making redudant stripslashes. From the security books, I get that get_magic_quotes_gpc is something we don't want to rely on. While this may add some security to badly coded scripts, it adds a false sense of security and promotes bad coding.

There are two approaches here:

1) phpBB uses the following: all input is magic_quotes_gpc'd - if not, there is a manual process to make them addslashed.

+ Pros

* Protection on badly coded scripts
* No extra work to add variables to queries for database security concerns

- Cons

* Each variable needs to be tracked where it comes from, and stripslash'd when displayed directly back to browser (such as on a search form)
* Further validation functions needs to take into account any magic_quotes_gpc'd variables
* No immediate code changes need to be done in the query line - may not be obvious that there are security issues

2) The opposite approach: input variables are coming in clean and unsanitized. If get_magic_quotes_gpc is used, reverse it

+ Pros

* The obvious code is added before it is being used (such as in a query), ensuring that all lines use the proper escape
* Variables can be passed for other validation (such as check_html) first without mudging the actual data
* A bunch of stripslashes is not need to make sure it is "database safe" - rather only one at the query line

- Cons
* Variables are not automatically safe to be entered in the database
* All lines must contain the proper escape

My suggestion is the second approach. I am testing code to undo get_magic_quotes_gpc and making sure it works in all cases. That would make the stripslashes in check_html unnecessary. Finally, it would put the burden to using addslashes (or mysql_escape_string) on the query lines... which is what chatserv has been doing at any rate.

I believe this will fix further issues with duplicate slashes and fit in with what the security books recommend.
We do need to discuss issues of filtering (allowing admins control of what is escaped, but that maybe for another post).

Regular Joined: Jun 02, 2004 Posts: 88

Looking to the future, I would think bypassing magic_qoutes_gpc would be the option since it has been reported to be deprecated in PHP 6. Creating some kind of function that tests for magic_quotes_gpc and then stripslashes if it is turned on should get consideration.

It appears more due diligence should be paid to filtering and escaping inputs and then escaping outputs. Also, initializing all variables in functions to ensure proper data is placed into the database.

Along those lines a consistant programming method would have merit since some things are done a bit differently. Maybe there could be one website that contains tutorials on how to properly secure 3rd party modules and similar issues. And no, I do not mean for someone to just go try to find something in a forum by guessing what keywords to search for...... Wink

Part of the tutorial system could be how to make themes, modules, blocks and CSS compliant to known web standards.

I guess what I am trying to say is now that the fury is over from putting some kind of security into the PHP Nuke code, maybe it is time to put into tutorials why certain things were done, why certain things were changed in patch files, etc. It is not easy for most people to breeze thru code and fully comprehend what is done and for what reasons.... Tutorials could help some people work on 3rd party modules or blocks they may have in place. Maybe this can encourage future php coders to write good code from the start.

From your vote choices, I prefer 3) ...... with bypassing magic_quotes_gpc, cleaning input variables, filtering inputs, and then using mysql_real_escape_string() but then I am prejudice to using only MySQL. Smile

Hope to see other opinions here.....

Posted: Wed Aug 30, 2006 2:53 am

I need to do more reading before I can comment on this one.

Posted: Wed Aug 30, 2006 6:33 am

Here are some debating issue links on addslashes vs. mysql_real_escape_string:

Chris Shiflett--
http://shiflett.org/archive/184

Ilia Alshanetsky--
http://ilia.ws/archives/103-mysql_real_escape_string-versus-Prepared-Statements.html

Both are considered PHP experts and both have books out on PHP Security. Smile

and yes, they disagree sometimes..... Wink

registered · Posted: Wed Aug 30, 2006 9:13 am

I am unsure why you want to reinvent the wheel in this case. There are plenty of well tested, and well used scripts to provide input filtering. Why not look into using one of them.

Posted: Wed Aug 30, 2006 11:56 am

Well this thread was just to consider where input filtering occurs, not saying that get_magic_quotes_gpc is adequate protection. I am, of course, supporting spottedhog's options that we will need to work on filtering inputs better.

That will bring the next discussion on such options for input filering such as kses and others Smile

Posted: Thu Aug 31, 2006 11:13 am

Going to seperate into a seperate thread

registered · Posted: Fri Sep 01, 2006 8:52 am

After reading the links posted by spottedhog (Thanks!), the question becomes is this really an issue with PHP-Nuke? I honestly do not have ANY experience with character sets. Does nuke, out of the box, support multiple MySQL character sets?