Writing Custom Module - SQL Questions

Worker Joined: Jan 22, 2007 Posts: 104

I'm writing a custom module that will allow users to enter data that will be put into the database. I know there are all kinds of SQL Injection problems with PHP-Nuke. What do I need to do in order to avoid possible database hacks?

Are there particular PHP methods that should be used that negate the SQL Injection?

In Java SQL Injection really isn't a problem due to the use of Prepared Statements. Is it possible to do this in PHP?

Are there PHPNuke or sentinel methods that are used to scrub the SQL text before it's put into the database?

Thanks for the help!

Posted: Sat Jan 12, 2008 4:37 pm

There isn't anything unusual you need to do for PHP-Nuke, just follow normal PHP programming precautions. Validate and sanitize your inputs as much as possible. Use addslashes() or mysql_real_escape_string() on strings before inserting into the database. If inputs should be numeric, convert them before inserting them (e.g. intval()).

The mysqli library present in PHP5 lets you do prepared statements, but Nuke does not support that. You could of course use it yourself, but your module might not work on those with PHP4.

The check_html() function is a Nuke function that is used to remove HTML from input text.

Posted: Sat Jan 12, 2008 4:43 pm

Thanks! So a simple call to check_html() and then addslashes() for all input text and converting numbers to intval() (which should be fine for the numbers I'll be using) will ensure that I avoid any/all SQL Injection problems?

That seems pretty straight forward and easy to follow. Why has PHPNuke been so hackable in the past?

Posted: Sat Jan 12, 2008 4:59 pm

valdarez wrote:

Thanks! So a simple call to check_html() and then addslashes() for all input text and converting numbers to intval() (which should be fine for the numbers I'll be using) will ensure that I avoid any/all SQL Injection problems?

Yes, that will help a lot. Just make sure you do it consistently and on every user input.

valdarez wrote:

That seems pretty straight forward and easy to follow. Why has PHPNuke been so hackable in the past?

Ha-ha. You are setting me up for quite a joke. I'll just diplomatically say that the author of PHP-Nuke really doesn't know what he is doing.

Posted: Sat Jan 12, 2008 7:47 pm

I've been studying up on this situation and will just elaborate briefly on what Gremmie said. You can find many threads on filtering in these forums that go into the topic in more detail. And if you google html filtering or something similar you can find a bunch of great information. Wikipedia is a good starting point and that has a bunch of links.

Basically Nuke is made up of a bunch of forms and programs that process them and put the data into MYSQL. So you might have an input field with the name "dog" on it. If you look in mainfile.php you will see the crutch that the author of Nuke relied on:

Code:

if (!ini_get('register_globals')) {


    @import_request_variables('GPC', '');


}

This gives the programs that process the form access to the POST variables from the form, just by using the variable name ... e.g., $dog. Often that's what the author of Nuke did. Instead of explicitly accessing the variable using $dog=somefilters($_POST['dog']) where somefilters could be intval or htmlentitities or check_html, he often just relies on $dog to be valid data. There is no standard practice throughout Nuke, in one case he may run check_html on the data, in other cases he may do an intval but you can't count on anything consistent. Furthermore because the variables are not explicitly posted, when you are trying to maintain these programs or improve them it is maddening to try to figure out where the variables came from ... did they come from a form or not?

Proper design really starts with the form. Decide what possible values an input can have. Is it an integer? What's the range of values it can take? Filter to make sure it fits in that range. Is it text? How long can it be? You can set the maxlength attribute in the form, then filter to make sure it's not exceeded. Can there be html in the input? Filter to make sure that only that which you want to permit gets thru.

Sentinel does provide an extra level of protection, particularly against cross site scripting and union and sql injection type of attacks but your first level should be within your program. If Sentinel has to come into play then there is likely something wrong with your program in the first place.

Posted: Sat Jan 12, 2008 9:00 pm

Yeah, coding with register globals on is really dumb. To be fair, it's possible that when PHP-Nuke was started, that was standard practice across the board. But no attempt has been made to correct it.
[ Only registered users can see links on this board! Get registered or login! ]

Wink

Site Admin Joined: Jun 04, 2004 Posts: 6432

In addition to validating the form, there are instances where variables are passed through links (ie in the URL). These must also be validated using the techniques above.

Posted: Sun Jan 13, 2008 12:14 pm

Sounds like he just didn't follow good coding principles. In Java (sorry, I've coded Java since the JDK 1.02, for well over a decade now, it's pretty much all I know, lol) particularly, when coding using a connection pool, you must close the connection after every call in order to return the connection back to the pool. I can't tell you how many systems I have worked on where developers have forgotten to close the connection, even after we provided a specific template that had to be followed.

I really like PHP, it's pretty straight forward, the API manuals are great, because people sound off on them and provide real world examples. The only downside for me is that I'm a backend developer who misses formal 'classes', but it's really the HTML, Javascript, and other front end UI stuff that takes me forever to develop / test. Although, now that I have found FireBug, even that is getting easier. Smile

Posted: Sun Jan 13, 2008 12:28 pm

kguske wrote:

In addition to validating the form, there are instances where variables are passed through links (ie in the URL). These must also be validated using the techniques above.

Yes, all user inputs must be validated, whether they come from $_GET, $_POST, $_COOKIE, etc.

Posted: Sun Jan 13, 2008 12:31 pm

valdarez wrote:

Sounds like he just didn't follow good coding principles.

Well he claims on his website he learned all he needed to know about PHP in a week or something, and it shows. Security was the last thing on his mind.

valdarez wrote:

The only downside for me is that I'm a backend developer who misses formal 'classes',

Classes as in OO? PHP4 has limited support for classes, and PHP5 has really improved things. I would like to design an OO version of Nuke in PHP5.

Posted: Sun Jan 13, 2008 6:41 pm

Gremmie wrote:

I would like to design an OO version of Nuke in PHP5.

OK, can you have it done by the 27th killing me

Posted: Sun Jan 13, 2008 7:01 pm

I'd like to see a Java version of Nuke. JBoss and a couple of others have tried to start OS solutions, but nothing has really taken off, and the solutions that are available are limited or fairly buggy.

Posted: Sun Jan 13, 2008 8:20 pm

Guardian2003 wrote:

Gremmie wrote:

I would like to design an OO version of Nuke in PHP5.

OK, can you have it done by the 27th killing me

I'm on it. Razz

Posted: Mon Jan 14, 2008 12:59 pm

... and, get yourself some really good books on PHP Security (you can never read enough of them IMO). These are some of my "favorites":
[ Only registered users can see links on this board! Get registered or login! ]
[ Only registered users can see links on this board! Get registered or login! ]
[ Only registered users can see links on this board! Get registered or login! ]

Happy reading! Smile

Posted: Mon Jan 14, 2008 3:17 pm

I only want an admin to be able to run these SQL statements, or to view this page. Is a simple call such as is_admin($admin) enough to ensure that nobody else can access this functionality?

if(is_admin($admin))
{
// My Custom Code
}

Posted: Tue Jan 15, 2008 6:14 am

I personally believe so and have used it as such quite a bit. Plus, with the upcoming RN release (2.2.0), we have fixed up is_admin() and is_user() a bit to make it run faster... trying to shave off every little microsecond(s) that we can... lol.