Page 1 of 1

passing values in the url and protecting against injection

Posted: Mon Aug 04, 2008 4:13 am
by Wilbo
Hi there,
I'm a noob with php security so I need some advice.
Lets say I have a News page on my site and when you click on the article title, the id of the news article is passed through the url and you can read the whole news article - simple stuff.

My question is, is this open to attack? Do I have to validate the value passed through the url?
And if so how do I do this?
The value passed would be an int.

Thanks in advance.

Wilbo

Re: passing values in the url and protecting against injection

Posted: Mon Aug 04, 2008 4:19 am
by jaoudestudios
Well you could do a few things.

Most importantly is mysql_real_escape_string when you put this value into the mysql query!

You could cast the int value as an int to receiving it to make sure, also if the value is not found in the database, so if they modify the value to something that does not exist in the database, have a fail safe page or redirect back to make list of articles.

Re: passing values in the url and protecting against injection

Posted: Mon Aug 04, 2008 4:37 am
by jaoudestudios
This might help, a bit over kill as you dont need the encode, but wont do any harm in having it.
http://www.forum.jaoudestudios.com/view ... f=13&t=103

Re: passing values in the url and protecting against injection

Posted: Fri Aug 08, 2008 4:38 am
by Mordred
jaoudestudios, your functions are wrong on several levels. I don't have the time to rebuke your code right now, so I won't go into details, see if you can figure it out by yourself :)

Wilbo:
In order to ensure data integrity and security in SQL queries, you need basically two things: mysql_real_escape_string() and quotes around values. There are other specific points of failure, which I've described in this paper:
http://www.logris.org/security/the-unex ... -injection
It is a bit advanced, in the sense that it expects the reader to know the basics of the SQL injection attack. Start with the references section at the bottom, which lists some papers on the basics.

Re: passing values in the url and protecting against injection

Posted: Fri Aug 08, 2008 6:23 am
by jaoudestudios
The function was not designed to go around the entire query, but each php variable in the query (may be I should have made that more clear)
ie...

Code: Select all

 
$q = "INSERT INTO table SET id = null, name = '".db_safe($name)."'";
 
But I did not realise that mysql_real_escape_string was not enough! I will read your article and let you know my thoughts.

Re: passing values in the url and protecting against injection

Posted: Sat Aug 09, 2008 4:00 pm
by Mordred
jaoudestudios wrote: // 1. remove white spaces (trim)
// 2. remove any html (strip_tags)
// 3. remove any escape slashes put infront of quotes by magic quotes (stripslashes)
// 4. encode all special characters i.e. double quotes, single quotes, ampersands, symbols etc (htmlentities) - no need to decode when pulling data out of database. browser will automatically use doctype to decode
// 5. escape any data that could harm database
Comments:
1. Don't. (Who are you to trim my whitespace, Mr. "db escaping function"!)
2. Don't. (Who are you to remove my HTML, Mr. "db escaping function"!)
3. Don't. (Who are you to deal with magic quotes, Mr. "db escaping function"! -- Article)
4. Don't. (Who are you to encode my HTML entities, Mr. "db escaping function"!)
5. Do, but don't forget to quote properly etc. (cf. the paper in the post above) (Ah, well done, Mr. "db escaping function")

So, you're about 20% right. Not too impressive when we're talking about security :/

Re: passing values in the url and protecting against injection

Posted: Sun Aug 10, 2008 10:14 am
by jaoudestudios
I see your point, but...

1. From my experience when users are entering information, i.e. email address, if they copy and paste it from somewhere it usually has a space on the end, which is not good when they want to register or log back in as it might not match if they type it unless they put the white space on at the end again - however, they probably dont even know it is there in the first place! So I strip it!
2. I dont want the user to enter html, I will style their information the way I want it styled! They can only enter plain text, hence I remove html!
3. I dont want quotes escaped! (I will encode them later on!!!)
4. I was everything encoded so that no code breaks
5. As a pre-caution I must escape if anything gets through.

So your reply...
(Who are you to trim my whitespace, Mr. "db escaping function"!)
...does not really satisfy the reason to remove this functionality.

So it seems you are 80% wrong. At least we agree on 20% :)

Re: passing values in the url and protecting against injection

Posted: Mon Aug 11, 2008 5:18 pm
by Mordred
jaoudestudios wrote:I see your point, but...

1. From my experience when users are entering information, i.e. email address, if they copy and paste it from somewhere it usually has a space on the end, which is not good when they want to register or log back in as it might not match if they type it unless they put the white space on at the end again - however, they probably dont even know it is there in the first place! So I strip it!
2. I dont want the user to enter html, I will style their information the way I want it styled! They can only enter plain text, hence I remove html!
3. I dont want quotes escaped! (I will encode them later on!!!)
4. I was everything encoded so that no code breaks
5. As a pre-caution I must escape if anything gets through.

So your reply...
(Who are you to trim my whitespace, Mr. "db escaping function"!)
...does not really satisfy the reason to remove this functionality.

So it seems you are 80% wrong. At least we agree on 20% :)
1. Trimming whitespace is dubious, but even for valid reasons, like the email example you give, the place for such cleaning is in the form handling code, not your database escape function
2. Many things are wrong:
- strip_tags() has issues, read what Ambush Commander wrote in his HtmlPurifier project.
- point 4 takes care of that (I will refute point 4 below)
- XSS happens inside tags too, which strip_tags will miss
3. You haven't read my article. It explains why you're wrong.
4. Aaah, the beloved html entities encoding in the database code. It's wrong. Do it before output, not before database entry.
5. "if anything gets through"? No, you must do this step always, nothing much, nothing less.

I'm not always right, but I prefer being proved wrong with arguments (like I do). These were not valid arguments, just statements of what you want. You didn't explain why you think they are correct (and I did explain why they are wrong).

Re: passing values in the url and protecting against injection

Posted: Wed Aug 13, 2008 8:10 pm
by The_Anomaly
jaoudestudios wrote:Well you could do a few things.

Most importantly is mysql_real_escape_string when you put this value into the mysql query!

You could cast the int value as an int to receiving it to make sure, also if the value is not found in the database, so if they modify the value to something that does not exist in the database, have a fail safe page or redirect back to make list of articles.
Couldn't he also use PDO as opposed to the mysql functions? I ask this mostly because I've been wondering if PDO automatically escapes my variables when I use the prepare(), then bindParam(), then execute() procedures. Does this perform the equivalent of mysql_real_escape_string?

This didn't seem worth starting another thread--and the answer would provide an answer for the OP as well, so I hope this isn't considered hijacking the thread.

Re: passing values in the url and protecting against injection

Posted: Thu Aug 14, 2008 1:02 am
by eskio
Lets say I have a News page on my site and when you click on the article title, the id of the news article is passed through the url and you can read the whole news article - simple stuff.
if id is numeric you can check simply like

Code: Select all

if (!is_numeric($id)) exit;