Validation of user submitted (tainted) data

A good example of why you should ALWAYS validate values before using them - treat $_POST as containing junk or worse and only copy vales out of that array when you have confirmed they are valid.

Moderator Note: This topic was split off from this thread: Delete specific row in database table - #14 by Mittineague

@felgall
That’s actually a bad example. A database should be able to hold any junk, no matter if you call it “valid” or “invalid”.
Whatever validation is irrelevant to database interaction. And your database layer have to accept any sort of data.

1 Like

Why???

@gandalf458 because it’s a database?

Yeah, so it should hold data, not junk

@gandalf458 at the moment sitepoint database holds several instances of 1 OR 1 = 1 “junk”.
So you going to say it shouldn’t be doing so?

several instances can easily be turned into several trillion instances - at which point it would then be almost impossible to find any actual data in the database.

swamping a database with junk can be just as effective at disrupting a web site as any other form of attack.

Come on, dude, your rant is irrelevant to the question.
especially regarding several trillion instances which your “array with confirmed data” is unable to prevent anyway.

The question with 1 OR 1 = 1 is a question of SQL injection. And your validation has nothing to do with it.
So, validation is just off topic here.

Late to the party, but @colshrapnel would you have been more open to the prior suggestion if it referred to santization instead of validation to prevent 1 OR 1 = 1 from going into that DELETE statement?

Either way, suggesting to use $_POST[‘whatever’] without ensuring it won’t do harm (especially on a DELETE statement) is horrible advice.

@Valentin_Gjorgoski, I encourage you to make cast your $_POST[‘id’] to an int or verify it is an int before sending it as a parameter to your DELETE statement (same for $_GET[‘id’], if you choose to go that route)

2 Likes

@cpradio NO
“sanitization” is even worse than validation, as nobody knows what the hell sanitization is.

Instead of using these vague and unreliable terms, the OP should be using prepared statements, which are straight and unambiguous way to handle database interactions safely.

2 Likes

OK… guess many should go back to class then. :smile: I have no doubts as to what santization and validation mean.

Except he was using them, just improperly. At least santizing the value would have made that point (his improper usage of them) moot.

@cpradio the problem with “sanitization” is that it’s rules are vague and inconsistent.

I don’t understand you guys. Instead of telling him how to use prepared statement properly, which will save his ass in 100% cases, you prefer ranting of sanitization which you cannot even define. Well, after all it’s PHP section, so I shouldn’t be wondering.

Why? If you expect an int, specify it as an INT (hint, you can do this within prepared statements too), a string, specify string, etc. Doesn’t seem inconsistent to me.

Didn’t you already do that? Then we got on the topic of validation/sanitization…

Granted, the method described in your post leaves one big wondering question, what type will PHP’s PDO cast your value to? Will it be an int, decimal, float, bool, or string? I think the answer is any and all of the above depending on what value it finds in $_POST[‘id’] (fairly certain that is true).

Curious as to which reply this was directed towards?

If it was

Then the record with an id of 1 one be deleted. Pretty simple.

No one in particular, just that I would do something more like this

function return_safe_int($user_input) {
  $protected_ints = range(1, 25); // or whatever, if any
  if ( is_numeric($user_input) 
      && (! in_array($user_input, $protected_ints) ) {
    return (integer)$user_input;
  }
  return false;
}

    $safe_id = return_safe_int($_GET['ID']};
    if ($safe_id !== false) { 
       $query = 'DELETE * FROM studentraspored WHERE ID = :id';
       $pdoStmt = $db->prepare($query);
       $pdoStmt->bindParam(':id', $safe_id, PDO::PARAM_INT);
       $pdoExec = $pdoStmt->execute();
   }

BTW @Valentin_Gjorgoski depending on what you mean by “not working” is by any chance the field not uppercase “ID” ?

“Come on dude”. You’ve got to be kidding me. I pray to god that you don’t use these on a live server. PHP will always return the array and make if($_POST) true no matter if you made up that lie. I sent this as a json to show you that I’m not making up lies like you are. No matter if the information is present, it’ll still result in true when the information isn’t.

“Come on dude”. Now it just sounds like you’re trolling. If the data doesn’t exist in the database and you don’t do proper error handling, you’ll leave your users with a blank white page with nothing or an infinite loop which will most likely crash the page. How are you going to tell the user that the page they are looking for doesn’t exist in your database? It’s hard to believe someone with a PHP background like yours would give amature and nooby codes like this to people asking for help.

To continue the Off-Topic discussion, It’s easy enough to test.

Doing a simple var_dump($_POST) shows the first before a submit, the second, after a submit.

.

True or False?

if ($_POST) { var_dump($_POST); } else { echo "false"; }

The string “false” before submit, the var_dump after submit

Chill out guys, no need to fight so much over things that boil down to personal preferences or specific use cases. Sanitization is such a thing because there is no strict definition of what it really does apart from making the data ‘safe’. Whether you want to strictly validate the input or allow somewhat invalid input but sanitized later on (like “1 OR 1” => 1) is a matter of individual use case - sometimes you need to guarantee best security and validate as strictly as possible whereas sometimes there is little gain in doing so and simply sanitizing (like casting to int) will suffice. The most important thing is that the application stays secure.

Wrong - your case is different since you are sending empty fields in POST so the array is not empty any more because it contains 2 fields - and the fact that they are empty or not does not matter. However, if ($_POST) can be used to check whether the form has been submitted or not - instead of checking for $_SERVER['REQUEST_METHOD']. It is not exactly the same but in the majority of cases if ($_POST) will be enough as a shorter method. There may be a case of a POST request with an empty form but in practice it happens very rarely.

Anyway, I prefer if (empty($_POST)) so as not to trigger notice errors. Edit: I was wrong about the notice errors - $_POST is always set in PHP so using empty() is not necessary.

Obviously, this cannot be used to check if individual posted fields are empty or not - only to check if the form has been submitted (hint: $_POST will always evaluate to false for GET and HEAD requests).

1 Like

Sanitizing is t he removal of all invalid characters in the value so that you know for certain that the value can be valid.

So if a value is supposed to contain a number then sanitizing will strip out anything that isn’t a number.

The reason this issue keeps getting raised is that people keep posting incomplete code with the validation/sanitizing code missing. By all means just post the code you are working on and leave out that part but you shouldn’t be substitutiong tainted values for the untainted ones that the code actually uses.

$_POST etc are tainted in that there is no way to tell in a properly coded program whether the value is valid or not

1 Like

@cpradio

specify string, etc.

See, this “etc.” is a problem. You still unable to give a complete definition. What your fellow developer have to do with this “etc.”? How to implement it in the code? What does “specify string” mean? That’s what “vague” and “inconsistent” mean. That’s why you helps noone telling them “to sanitize” something.

The method described in my answer is considered the only proper way in the entire world, but, as it turns out, totally unknown on Sitepoint.