Module Security Best Practices

Worker Joined: Dec 08, 2005 Posts: 107

Hi All!

I am in the process of cleaning up the code for several self-written modules I have. One important aspect of this is to increase the overall security of the modules. There are a number of "best practices" for any software project, as well as specific issue for internet applications that I have been implementing.

As part of this, I was looking for something that was specific to PHNuke modules. There are a number of useful comments and suggestions by people in these forums, but I was wondering if there was any list of things to look out for, things to do, and so forth. Does anyone have a list or can point me somewhere? Any help would be greatly appreaciated.

regards,

jimmo

Posted: Mon Apr 17, 2006 2:09 pm

This is a short page of things to consider [ Only registered users can see links on this board! Get registered or login! ]

It is in-no-way a reference to every security practice out there. I suggest a good PHP security book for that. But hopefully will give you some ideas as related to phpNuke.

It hasn't been updated with the most recent version (due to my server CVS being worked on).. but I've changed:

stripslashes - should not be necessary, only in few cases where magic_quotes_gpc is on. Recommend using check_html instead
FixQuotes - I don't recommend using this at all, better to use addslashes

Hope that helps

Site Admin Joined: Jun 04, 2004 Posts: 6432

It's an excellent reference - definitely a great start. Thanks, evaders99!

Posted: Tue Apr 18, 2006 1:00 am

Cool. Thanks! It taught me a couple of nice things and that is always a happy thing.

I have a couple of books that I am using, both specifically PHP (e.g. "Pro PHP Security" from Apress) coding and software security (forgot the name). However, the PHPNuke specific stuff is missing, so I really appreaciate the tips you provided. I would also appreaciate an update when the latest version is online.

I would like to see an explanation of each aspect (or a little more). That is, why it is a "good practice". For the most part I could figure them out. For example, "Defined constants are harder to modify than global variables". That is clear why to use defines and not globals (something I had not thought about)

However, under "Variables need to be validated before use", you say "Using single quotes around variables in SQL statements". This was something I can easily make the connection to when you refer to SQL injection, however it might not be immediately obvious to everyone. Not everyone know what SQL injection is and it wasn't immediately obvious (to me at any rate). I finally groked it, but it would still be nice to have a little more detail.

I would also like to contribute a little (assuming what I have to say is not in the updated version). One thing I did this past weekend was to to unify my code and reduce redundancies. For example, I had a couple dozen functions that were all calling mysql_query(). Now, each calls a single function which can then check for improper elements in the query. I also have a separate database for my stuff and am working on changing the functions to use a read-only most of the time and only use the write-user when necessary. (I have more, I just don't have access to my new code at the moment.)

Thanks again!

jimmo

Posted: Tue Apr 18, 2006 7:22 am

Sure - I'll be updating it as I can. Thanxs for your suggestions

Posted: Tue Apr 18, 2006 8:27 am

You also might want to throw in $db->sql_freeresult so people start using it Wink

I dont know if this made it into the last patched but

Code:

function define_once($constant, $value) {


    if(!defined($constant)) {


        define($constant, $value);


    }


}

Should help keep the defines from being a problem

Posted: Tue Apr 18, 2006 9:56 am

I wasn't aware of the sql_freeresult function, but it definately looks like it would be useful. I don't yet, have a large database, so memory is not an issue.

On another note, I was trying to find the place in the code where the query string is pased to catch things like sql injection and the god-user insert. Can someone point me to the right place?

Posted: Tue Apr 18, 2006 10:07 am

Freeresult frees a query result so it makes no difference how big your DB, but how big your querry is. Lets take a site that 6,000 users and you do SELECT * on it, imagine how big that query is. By default the garbage collector will clean up the memory when the session is done, but thats not good programming. Plus if you did that lets say 20 times, just think of the amount of memory you are using. You should be freeing it up when possible.

Everything runs through the mainfile. So if you really want to clean up or check for injections thats the place to do it. Or include a file to do it like sentinel.

preg_match is the best thing to use I think in those situations.

Posted: Tue Apr 18, 2006 11:46 pm

In my case the queries aren't go to be very big because of the relatively small database, but I understand the difference between a small result and a small database. Granted a query on all 6000 users (or even 3500 in my case) would likely take up more memory that a single row on a database with millions of records. But, here I am missing a little knowledge. Granted the programmer should do the clean-up (definately helps avoid memory leaks, among other things). The questions is just when is the "session done"?

I was already considering something like preg_match. What I was looking for is some regex's so that I didn't have to reinvent the wheel.

Posted: Wed Apr 19, 2006 9:47 am

Sure, but if you dont do it now and forget to do it later when your site is larger Smile

I have a few regex's I am testing. But I am not sure they are 100% so I dont want to give them out yet.

Posted: Wed Apr 19, 2006 10:33 am

Very valid point. Fixing problems is always harder than doing it right the first time.

I can appreaciate not wanted to give out any code until you are happy with it. I'm just lazy. Wink

Posted: Wed Apr 19, 2006 12:18 pm

You might find this very helpful, I know I did [ Only registered users can see links on this board! Get registered or login! ]

Posted: Wed Apr 19, 2006 12:25 pm

Thanks! Definately could save a bit of time.