Page 3 of 4

Posted: Fri Oct 21, 2005 5:32 am
by Jenk
I'm banging my head on my desk here.. you have completely changed your argument.

You said you must add multiple checks to ensure the input is filtered. THAT is superfluous.

For your informatrion, as you obviously haven't worked it out yet, intval is a perfectly safe way to ensure the input is clear of all 'dangerous' characters, unless MySQL have suddenly decided that some Int char's are special chars..

At no point in my example or anywhere else is only some input escaped, if you care to read up on the function intval() you will realise that any char's not equivocal to an Int are removed, i.e. if it aint 0-9, it doesn't get returned.

So.. if you know the input should be an int (and ID field is a perfect example..) then using intval is a perfectly acceptable method to filter, validate and escape, all in once step ;) After all, in an integer value has nothing to escape with a slash, does it? So what good does using mysql_real_escape_string or any regex do? ;)

Code, such as escaping an integer value, after it has been filtered by intval() is superfluous, thus not optimal.

BTW - you do not 'prepare' for developers cock ups. If they are going to remove the intval(), they could do any manner of cock ups, which none are my 'fault' (as that is what you seem more worried about).

I've never, ever seen a developer add in extra code just to satisfy the incompetence of possible future developers, because, like I said, it is poor practice to include extra code just for this purpose. This is what the documentation and comments are for.

Posted: Fri Oct 21, 2005 8:11 am
by Maugrim_The_Reaper
I'm banging my head on my desk here.. you have completely changed your argument.
?
You said you must add multiple checks to ensure the input is filtered. THAT is superfluous.
??
For your informatrion, as you obviously haven't worked it out yet, intval is a perfectly safe way to ensure the input is clear of all 'dangerous' characters, unless MySQL have suddenly decided that some Int char's are special chars..
Trying to be as clear as possible. Intval is not escaping - it escapes nothing. Intval is Input Filtering, as it removes/negates an input value which does meet a predefined rule, i.e. must be an integer. Escaping and Input Filtering are two different things. When I mention escaping I am in no way referring to input filtering...if that's whats somehow gotten confused...?

In many applications you will filter input and validate, use it in your business logic, then perhaps write to the database. At the database level you escape (for XSS you might escape elsewhere of course). If IF and E are in separate layers it makes immediate sense for each to act independent of the other - Data Access couldn't give a toss about what's filtered so long as it can escape for any dangerous SQL Injection enabled characters and vice versa.

So input comes in - its a string supposed to be numeric. You filter and hit it with inval(). All is well with the world. You pass this to an SQL. It gets escaped. The Data Access class doesn't care whether you forgot a filtering rule, or it got edited by a confused developer, or was bypassed through some later change - it just escapes on the basis it's taking zero chances with user input.
At no point in my example or anywhere else is only some input escaped
What does inval() escape? I thought it just returned an integer value?
if you care to read up on the function intval() you will realise that any char's not equivocal to an Int are removed, i.e. if it aint 0-9, it doesn't get returned.
That's not escaping... And I'm perfectly familiar with intval() thank you very much... Please don't make assumptions about my PHP knowledge.
So.. if you know the input should be an int (and ID field is a perfect example..) then using intval is a perfectly acceptable method to filter, validate and escape, all in once step Wink After all, in an integer value has nothing to escape with a slash, does it? So what good does using mysql_real_escape_string or any regex do? Wink
Because Input Filtering != Escaping - they're two discrete tasks. They may be complementary, but in a well separated application they're not even close to being substitutes. I'll reserve a exception for procedural code whic is very rarely well separated like OOP practices.

Escaping + Input Filtering > Input Filtering, i.e. increased security (for whatever reason).
Code, such as escaping an integer value, after it has been filtered by intval() is superfluous, thus not optimal.
Its also less secure though - which is the point of the argument methinks, or methought.
BTW - you do not 'prepare' for developers cock ups. If they are going to remove the intval(), they could do any manner of cock ups, which none are my 'fault' (as that is what you seem more worried about).
You do - if you have to listen to the client whining at you because you fell into a support contract following your app development ;) I'm guessing type hinting will meet with similar disapproval...sigh. :)
I've never, ever seen a developer add in extra code just to satisfy the incompetence of possible future developers, because, like I said, it is poor practice to include extra code just for this purpose. This is what the documentation and comments are for.
So you will document that you don't escape *all* user input entering the database? Fair play in that case. ;) I know what you are driving at - I just can't agree. If I pull on my auditors cap for a moment all I see is the equivalent of deliberately lowering security over an essential area because an extra puny mysql_real_escape_string() call is not "optimal".

Defense/Security in Depth Principle promotes the ideal of never relying on one single security measure when two or more are implementable. Why? Because if one fails, you'll have another line of defence. It's a simple entirely reasonable concept.

Posted: Fri Oct 21, 2005 11:15 am
by Jenk
*sigh*

intval (not inval, intval.) is a perfectly acceptable and safe way to 'make safe' integers. Just how is ensuring an integer is an integer not safe? Exampkes please, not speculation and waffle about it not escaping, actual example.

Stop picking at straws, and stop being such a pedant.

Posted: Fri Oct 21, 2005 11:50 am
by Maugrim_The_Reaper
I'm not a pedant, I am a rambler. These are two very different things. Pedants are self righteous sods, Ramblers are simply incapable of writing in brief...;) If name slinging starts I'll be forced to drop my facade and start spelling everything in accordance with my accent - :)

That it not escapes is hardly "waffle"...but I digress, it's 5:30pm on a Friday and well, I have a pint of Guinness with my name on it...

Okay - you have an MVC (well, we'll call it well separated or altogether different debate will ensue). Your input comes in as a supposedly numeric value.

Step 1: Input Filtering
Step 2: Do business logic/validation/etc.
Step 3: save to database

All three are segregate layers - each an independent layer with little dependency.

Developer X pops along and starts fiddling with the Input Filter rules. He wrongly assumes that the application is well separated since Developer Y decided not to back up Input Filtering with Escaping in the Data Access layer (Step3). The Data Access Layer is now completely dependent on the Input Filtering three layers up the ladder.

So Developer X removes the intval() seeing little requirement for it - who knows why, indeed who cares. Maybe X simply drops the ball one day and does it completely by accident. It happens.

Unfortunately the Data Access Layer which was assumed to be responsible for escaping is not actually doing so. It had a complete dependency on the Input Filter, and that dependency now leaves it open to exploitation. So much for a well separated application...

No I'm not posting code - which is pointless since this is a very easy to understand concept. Yes, I'm rambling - yet again... This all revolves around whether or not errors (stupid or misinformed) occur. Unfortunately it does happen and I was only pointing out that defense in depth does have a basis in reality...

Posted: Fri Oct 21, 2005 1:09 pm
by Jenk
A pedant is someone who pays more attention to the minor (and often irrelevant) details, and not the big picture.

This is now an argument which is about nothing, I am stating that the use of intval on an integer value is a perfectly safe method to ensure it will not break your site/db. This is true, until DB system vendors start using integer values as special chars atleast, which isn't likely.

You are stating that there should be a second (atleast) level of filtering/validation/escaping/whatever to ensure that a future developer doesn't cock it up. I disagree with the 'idiot developer proofing', but your last post with the 3 stages of filtering, using and saving is what I do anyway.. so now I really do fail to see what this discussion is about?

Two different subjects all together.

Now, just to clarify, the example taken from my post on page 2 (of 'where do I use it')

Code: Select all

<?php

function sqlClean ($string) {
    if (get_magic_quotes_gpc()) {
        $string =stripslashes($string);
    }
    return mysql_real_escape_string($string);
}

$link = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Connection attmept to Database failed.');
mysql_select_db($db, $link) or die('Database Selection Failed.');

$itemID = (isset($_POST['itemID']) ? intval($_POST['itemID']) : ''); //this is an int
$item_name = (isset($_POST['name']) ? sqlClean($_POST['name']) : ''); //this is a string

mysql_query("SELECT * FROM items WHERE itemID = '$itemID' AND item_name='$item_name'") or die('Query failed.');

?>
As you can see, I have ammended to show that I use the 'escaping' methods immediately before I need to use the values in a query. Likewise, if I am outputting user variables to the user agent, I run it through htmlentities() right before I echo/print it. Usually in the form of:

Code: Select all

<?php
echo htmlentities($uservar);
?>
And my definition of documentation also includes comments. Drummed into me by EDS.

Hopefully that will clarify a point that was missed earlier..

Enjoy your Guiness.

Posted: Fri Oct 21, 2005 1:41 pm
by Charles256
i'm with you jenk. though i probably take it a step further..i simply check to see if the user tried to put anything funky into an input field and simply echo it back to the field and don't insert it into the database..some people say escape it or add slashes or do some crap and then insert it into the database..personally I say what the hell? why?! Just display it back and keep displaying it back until kiddy scripter realizes that <span style='color:blue' title='I&#39;m naughty, are you naughty?'>smurf</span> is not going to fly and enters something legit..but I could just be lazy... :-D

Posted: Sat Oct 22, 2005 11:40 am
by McGruff
Jenk: Chris Shifflett has a lot more experience than you do. You shouldn't dismiss his opinions out of hand.

This topic stands a high chance of getting locked unless the discussion moves forward instead of going round in circles as it is at the moment, to everyone's irritation.

Posted: Sat Oct 22, 2005 1:30 pm
by Jenk
McGruff wrote:Jenk: Chris Shifflett has a lot more experience than you do. You shouldn't dismiss his opinions out of hand.

This topic stands a high chance of getting locked unless the discussion moves forward instead of going round in circles as it is at the moment, to everyone's irritation.
Where have I dismissed his opinions out of hand?

EDIT: K, just re-read all his replies.

He has missed the point that the value type in the code, not the db, is an integer. If you intval a value in PHP, all you are left with is the integer value, that is a number. There are no chars left to escape, so there is no need to run it through a function such as mysql_real_escape_string(). The fact it is not a string should be enough to tell you that.

Posted: Sat Oct 22, 2005 4:33 pm
by McGruff
I can see your point. However, the defence in depth idea is worthwhile. Suppose the intval function had a bug (it wouldn't be the first php function which did) and that this was exploitable. Not likely perhaps but you never know. Things do go wrong. Another programmer could forget to make the check in the first place. It's exactly because things do go wrong that the idea of multiple, redundant checks is worth considering.

Posted: Sat Oct 22, 2005 4:46 pm
by Jenk
Ok, I see your point now.

EDIT: But still disagree. Superfluous coding is poor practice.

Posted: Sun Nov 13, 2005 3:25 am
by shiflett
Jenk wrote:He has missed the point that the value type in the code, not the db, is an integer.
No, I haven't. I'll state this once more, and then I give up. Here is the original question:
I've got a small question, do I need to use real_escape_string on post and get if the data type in the database is a integer.
Read that very slowly, and then read it again.

The answer to this question is yes, period. I think my original reply explains this pretty clearly.

Posted: Mon Nov 14, 2005 2:41 pm
by Ambush Commander

Code: Select all

$sql = "SELECT * FROM `table` WHERE id = {$_GET['id']}";
Absolutely bad.

Code: Select all

$id = (int) $_GET['id'];
$sql = "SELECT * FROM `table` WHERE id = $id";
Not security threat, but a fairly weak defense in depth. (defense_in_depth is not a boolean attribute)

Code: Select all

$id = mysql_real_escape_string((int) $_GET['id']);
$sql = "SELECT * FROM `table` WHERE id = $id";
Not security threat, but still not very good defense in depth.

A stronger defense in depth is implicit: you don't give the programmer a chance to mess things up.

Code: Select all

$id = (int) $_GET['id']; //note now the typecast is unnecessary but helpful
$sql = "SELECT * FROM `table` WHERE id = ?";
$db->execute($sql, array($id));
Not security threat and a reasonable defense in depth as long as $db implicitly handles escaping of input. Force all queries to go through this object, and if a programmer forgets that IDs are only integers, pass the string into the query and have it be escaped, whatever. Just'll turn up an empty result set.

Defense in depth advocates redundancy: once again, it is not a boolean (true/false) attribute.

Posted: Mon Nov 14, 2005 6:47 pm
by Jenk
shiflett wrote:
Jenk wrote:He has missed the point that the value type in the code, not the db, is an integer.
No, I haven't. I'll state this once more, and then I give up. Here is the original question:
I've got a small question, do I need to use real_escape_string on post and get if the data type in the database is a integer.
Read that very slowly, and then read it again.

The answer to this question is yes, period. I think my original reply explains this pretty clearly.
I've read it three times, on three seperate occasions.

I've also read your replies several times, on seperate occasions.

Your example:

Code: Select all

if (strval(intval($_POST['user_id'])) === $_POST['user_id'])
{
    /* $_POST['user_id'] is an integer */
}
Now with the exception of comparing it to the original value, which I will hold my hands up to about not suggesting/using, you've done exactly what I suggested anyway.

So apart from that.. what is different? You haven't used mysql_real_escape_string(), which is what I was suggesting is unecessary, and you have used intval to return only the numeric values of the expected integer, which is also what I was suggesting.

Seriously.. I don't understand here, is what I have been 'discussing' with maugrim previously all been over just been comparing to the original value? Surely not?

For class building, I understand the want/need to include escaping in your classes, keeping with the MVC/OO idea, so you can just transplant classes between projects and so on, but in a procedural fashion..



btw, I quesiton/argue when I want to understand, so please.. in as simple english as possible... why use mysql_real_escape_string() on an integer that has been passed through intval()? To this effect, the only argument so far that I have thought "ah, that is true" is McGruff mentioning the possibility of intval haveing a bug/exploit.

EDIT: I'd like to further add that I appreciate I am not the most experienced programmer in the world, and that others who are considered some of the most experienced and advanced programmers have all given much time and consideration to this very subject, but I am still left with the question "Why?" .. unfortunately I simply cannot settle with the "because" argument, it's a flaw in my character. :(

Posted: Mon Nov 14, 2005 8:40 pm
by Ambush Commander
For class building, I understand the want/need to include escaping in your classes, keeping with the MVC/OO idea, so you can just transplant classes between projects and so on, but in a procedural fashion..
OK, this is a bit crucial. Procedural is, by it's nature, a shallow way of looking at a problem. Now this isn't necessary bad, because shallow = fast, and we all know that time is money. But when the solution is shallow, there is less ability to have defense in depth. So you could argue that. And I won't dispute you. But I'll still see the writing on the wall.

Nevertheless, if you do go procedural, I would argue for using a DB mapper class. They are dead useful, and noodling around with the direct mysql_*() functions is a bad idea in my humble opinion.
btw, I quesiton/argue when I want to understand, so please.. in as simple english as possible... why use mysql_real_escape_string() on an integer that has been passed through intval()? To this effect, the only argument so far that I have thought "ah, that is true" is McGruff mentioning the possibility of intval haveing a bug/exploit.
When the variable is guaranteed to be an integer, there is no reason to at all. Absolutely none. That's because the variable is guaranteed to be an integer. In reality, it is not guaranteed to be an integer. Sure, you can argue:

Code: Select all

$value = (int) $_GET['id'];
That has to be an integer. And you're right. In that situation the value is guaranteed to be an integer.

But security is not about the security of each situation. Security of software is the security of all situations, and if one situation is missed (as has happened with many, many other applications), the entire application is compromised.

This is what Maugrim_The_Reaper tried to argue. However, it is misconstrued as "some developer haphazardly removes the checking" and then Maugrim accepts it as a possibility, and the point is lost.

Let me rephrase the situation in a different way.

In PoEAA, Fowler discusses Implicit Locks. In many ways, locks are similar to security. If something is locked/secured anywhere it must be locked/secured everywhere. Locks are a bit less intuitive, so you may think that the risk a lock is forgotten is higher. And yes, it is higher. Filtering data is something that you probably don't think much about anymore, you just do it.

But are you willing to let one mistake cost the security of your application?

Posted: Mon Nov 14, 2005 9:31 pm
by shiflett
(Note: My previous reply sounds a bit hostile, but it is not meant to be. I am sometimes brief and to the point.)

There are two completely separate discussions taking place, and it's important that they not be mixed.

1. Is escaping necessary when a field in the database is defined as an integer?
2. Is escaping necessary when a variable is an integer?

The original question falls into the first category, and the answer is not subjective. I've already given a very specific reason why in my first response, and I used an SQL statement and SQL injection attack to illustrate the vulnerability. The database schema cannot protect against SQL injection - that's not what the attack targets.

The answer to the second question is not as objective, and I can appreciate that some people won't see the value in implementing redundant safeguards or adhering to other theoretically secure practices. Often, the value has as much to do with the social aspects of software development as the technical ones.

So that we don't risk confusing the original issue further, I think we should move any discussion that falls into the second category to its own topic.

P.S. I should have mentioned ctype_digit() as a better alternative for ensuring an element in $_POST is an integer.