Page 2 of 4
Posted: Mon Oct 17, 2005 3:36 am
by Maugrim_The_Reaper
Is it necessary to escape an numeric value? No.
Is it a good idea to escape an numeric value? Yes.
Why?
a) Defense in Depth. There is no OTT in security measures. Every (even the unintuitive) little bit helps. Take a C/R Login - how many have actually used one despite the possible benefits? How many add tokens to forms? How many know what XSS stands for?

Pretty few I guess... (hey, no finger pointing!)
b) Filtering != Escaping. They are complements, not substitutes.
c) Who says its an integer? You, the client, or God? All it takes is a small error you don't spot and suddenly that integer stops getting filtered. They call 'em bugs.
d) Its not an integer, in fact it never IS an integer (except for casting temporarily). It's a string. You never trust strings from a user. You filter them, escape them, and assume they just might still be bad evil gits...

e) When you can't see the entire picture it's simply logical. Take a well separated OOP app. Your input is filtered say somewhere at the start, the data is written 3 layers across in a Data Access layer.
Posted: Mon Oct 17, 2005 11:15 am
by Jenk
No you do not need to use mysql_real_escape_string() on an integer, and is a waste of time and resources if you do so.
If the value is an expected integer, use intval() instead.
Code: Select all
<?php
$var = intval($_GET['var']);
?>
Posted: Mon Oct 17, 2005 11:34 am
by Maugrim_The_Reaper
We can muck about until yonder cows come home...
Put it another way - where is your intval done. As part of Input Validation/Filtering, or as part of escaping procedures? If both are separated into two different files - is it wise to trust one file over another? What if someone removes your check (say a client or other developer) and you have added no additional escaping to cover the database code.
Where does that leave the security of the app as a whole?
Maybe our circumstances differ... I primarily write code for other people to eventually use and modify. So I just feel an instant need to cover every last base as far as possible.
Security is all about lowering risk - it'll never be perfect. Omitting a possibly unnecessary step which is useful in even limited circumstances does not lower SQL injection risk, it leaves it untouched. Why leave my security risk higher than it could be for any reason? That makes the app inherently "more" at risk of a security issue.
I think this is what Chris was trying to get across - from a purely security risk perspective, not adding that extra possibly unneeded step is equivalent to purposefully creating a security hole. Just because "something else" is covering it - it doesn't change the fact it still exists ready to bite your ass if someone removes the original cover intval() whether out of error, ignorance, or because they could see no real use for it (since all user input should be escaped anyway, saye he...

).
Another question is whether its acceptable to alter or fix user input. Some apps really won't want to - they might want that original input for a log of suspicious input, or something along those lines. Running an intval() would interfere in something like that - you've shifted from filtering/validation to "fixing".
Maug will now proceed to shut up rambling...
Posted: Mon Oct 17, 2005 11:46 am
by Jenk
If someone else removes my check, then it's their fault - I'm not about to add superfluous escaping/filtering just to satisfy the hap-hazardness of other developers incompetencies.
an example speaks better than words, so here is how I use it:
Code: Select all
<?php
function sqlClean ($string) {
if (get_magic_quotes_gpc()) {
$string =stripslashes($string);
}
return mysql_real_escape_string($string);
}
$itemID = (isset($_POST['itemID']) ? intval($_POST['itemID']) : ''); //this is an int
$item_name = (isset($_POST['name']) ? sqlClean($_POST['name']) : ''); //this is a string
//etc..
?>
Posted: Tue Oct 18, 2005 2:54 am
by Maugrim_The_Reaper
I'm not about to add superfluous escaping/filtering just to satisfy the hap-hazardness of other developers incompetencies. Razz
Unfortunately such haphazard developers are a reality

When a company IT department starts modifying third party software they are not to be entirely trusted. Unlike the original developers they won't know the code inside out...that makes them error prone. I've seen a few pretty ludicrous blunders in my career that drive home the point of making applications "idiot" proof...

Posted: Tue Oct 18, 2005 2:46 pm
by Jenk
The second they modify MY work, it is THEIR fault.
I WILL NOT add superfluous checks to my code, nor will any Developer worth their salt, just because someone might cock it up later on. It deminishes the performance of the code and makes YOU look a muppet.
"Idiot Proof" applies to users, not developers.
Sorry, but you are debating a void issue.
Besides.. if you are talking on Enterprise level, you are going to use such languages as ASP, which compiles and does not decompile very easily, thus a muppet is not likely to see the source code, or on a desktop app, Java, C++ or VB, where they are even less likely to see the source code - because you will, of course, have a competent VCS to prevent code getting passed around to Tom Dick and Harry, won't you?

Posted: Tue Oct 18, 2005 3:08 pm
by Ambush Commander
Hi Jenk.
Perhaps it would make more sense to have the database object that performs the query perform the input checking? Instead of checking directly when you retrieve the object from the $_GET, you pass the parameter around into the db object, it gets escaped, turns out it's a string, and your query silently returns nothing. You can't "forget."
Compare to concurrency systems. What if you forget to lock the row? You get some pretty bizarre errors. So make it implicit.
Posted: Tue Oct 18, 2005 4:05 pm
by Jenk
Hi, my example is in a procedural fashion. In an object, yes, polymorphism will take place.
Posted: Wed Oct 19, 2005 3:06 am
by Maugrim_The_Reaper
"Idiot Proof" applies to users, not developers.
The sword cuts both ways believe me...
The second they modify MY work, it is THEIR fault.
Unfortunately if it breaks they're going to play the blame game...
It's maybe an expectation gap. We expect other developers not to break it, the other developers expect us to exercise bug control. And they will view it as a bug if some auditor pops along, runs a few tests, and throws out such a winger... And the auditor will agree simply because they assume all user input will be escaped distinct from filtering - defense in depth principle - and if its not it represents a risk at some level. Oddly I happen to BE an auditor...
It's similar you know to the type casting debate. Personally I love type casting - it pushes responsibility for errors back to where it belongs - whoever started passing around unexpected objects in error. Unfortunately I seem to be in the minority on that score...

Posted: Wed Oct 19, 2005 9:14 pm
by Jenk
Sorry, but that's Bollocks.
So what happens if said idiot deletes a whole block of code, is it still your fault?
Posted: Wed Oct 19, 2005 9:24 pm
by Ambush Commander
One mans "idiot deleting a whole block of code" is anothers "careful refactoring that lead to undesirable repercussions due to integration errors".
Posted: Wed Oct 19, 2005 9:27 pm
by feyd
mmmmm, graceful degrading...
Posted: Thu Oct 20, 2005 2:41 am
by Maugrim_The_Reaper
One mans "idiot deleting a whole block of code" is anothers "careful refactoring that lead to undesirable repercussions due to integration errors".
Exactomundo...

Posted: Fri Oct 21, 2005 1:37 am
by Jenk
Bottom line is, you do not add superfluous code to take into account the incompetence of anyone who may edit your code at a later date.
It is poor practice, your code will inevitably not be optimised and you can NEVER make it safe from blunders from someone fiddling with it at a later date.
Once you have released your code, or sold it, you do so on a "this is what you get, I am not responsible for any problems that occur due to cockups from changes made to the code" basis.
Period.
So going along with the "One mans "idiot deleting a whole block of code" is anothers "careful refactoring that lead to undesirable repercussions due to integration errors"." quote.. what happens if said idiot removes a function block from your code? do you have it copied in a second time? no..
What do you do to ensure that a string is escaped for safe use in a MySQL DB? Do you run it through mysql_real_escape_string() twice? That won't work due to double escaping. So you use regex? That will be far less efficient.. So tell me maugrim, what do you do in such a situation?
Posted: Fri Oct 21, 2005 3:26 am
by Maugrim_The_Reaper
Bottom line is, you do not add superfluous code to take into account the incompetence of anyone who may edit your code at a later date.
It's not superfluous - it's a safety measure to push responsibility from you (a bug) to the incompetents (an error)

Some developers are required to support a product once developed - esp. if the client plans on modifying it in changing environments.
Either you escape all output (from user), or you escape some output. If you only escape some - you are leaving an open door protected by a distant obscure screen (Input Filtering). Whether the client, another developer, yourself or the US President removes that screen is irrelevant. They have influenced Input Filtering/Validation - not Escaping. Maybe they even have valid reasons to - you can't deny that's remotely possible. However in not bothering to escape what we know IS user input you have included an unnecessary gap in what should be a consistent concrete layer of protection without exceptions. There is little excuse for conciously hamstringing security by relying on a separate and distinct layer in the application. Sure its a nice logical act - but nice logical acts become nightmares if they are exceptions to the rule that no one expects.
This is a pretty common concept - Security in Depth. It's not like I'm the only person who'll hammer this point to death rather than back down an inch...
This is really one of those expectation gaps. Unfortunately not every developer will share your views - I don't, Chris Shiflett doesn't, and there will be hundreds lined up behind us...
It is poor practice, your code will inevitably not be optimised and you can NEVER make it safe from blunders from someone fiddling with it at a later date.
To borrow a phrase - bollocks. I may be a mad drunken Irishman, but Security in Depth is not what I would call poor practice. Its best practice.
Once you have released your code, or sold it, you do so on a "this is what you get, I am not responsible for any problems that occur due to cockups from changes made to the code" basis.
Some people like to see a support programme before jumping in with new applications. They may not have in-house developers, or may have a small group of in-house developers who don't specialise in your application. Sometimes the extra effort up front is far more time effective than having them haunting your shadow when the unexpected happens...
So going along with the "One mans "idiot deleting a whole block of code" is anothers "careful refactoring that lead to undesirable repercussions due to integration errors"." quote.. what happens if said idiot removes a function block from your code? do you have it copied in a second time? no..
That's an extreme example - if the function was removed it would produce an error (if used). Removing inval() while there being no escaping would produce what? An SQL Injection? An admin account being compromised? The end of the world as we know it?
What do you do to ensure that a string is escaped for safe use in a MySQL DB? Do you run it through mysql_real_escape_string() twice? That won't work due to double escaping. So you use regex? That will be far less efficient.. So tell me maugrim, what do you do in such a situation?
Filter all input
Validate all input
Escape all input (before entry to SQL)
All three, and each in an independent mutually exclusive process - no dependencies.
Why on Earth would I use mysql_real_escape_string() twice?
