Problems with space within parameters of a php function

Question

Hey,

Sorry for not formatting my code. the toolbar was gone...

I want to insert some data into a mysql db. I've wrote a function in php:

function add_ID($ID, $token)  {
 $add = "INSERT INTO ids (ID, token) VALUES ('$ID', '$token')";
 mysql_query($add);
 echo 'added successfully';
}  
if(isset($_GET['addDeviceID'])) {
 add_ID($_GET['ID'], $_GET['token']);
}

In the URL-Field of my Browswe I'am calling the function like that: http://www.justanexample.com/example.php?ID=123123123&token=qwertzuiop

That works.

If I put a space into either one of the parameters for example like that: http://www.justanexample.com/example.php?ID=123123 123&token=qwertzuiop

Nothing was added to my mysql db.

Would be great to get some help :) Thank you!

Answer 1

Remove space from them using str_replace function eg:

 $ID = str_replace(' ', '', $ID);
 $token= str_replace(' ', '', $token);

 $add = "INSERT INTO ids (ID, token) VALUES ('$ID', '$token')";

Also, I suspect your $ID is a integer field in your table so you can run your query without specifying quotes eg:

 $add = "INSERT INTO ids (ID, token) VALUES ($ID, '$token')";

Answer 2

You should validate your input before sending it to the database. Or, if validation is not possible, filter and/or escape the value.

Validation

If you expect ID to be a integer greater than zero:

if (!ctype_digit($ID)) {
    // invalid ID
}

If you expect token to be an alphanumeric string:

if (!ctype_alnum($token)) {
    // invalid token
}

Filtering

Filtering is removing invalid parts of the input so that it becomes valid:

if (!ctype_digit($ID)) {
    $ID = preg_replace('/\D+/', '', $ID);
    // $ID does now only contain digits
}
if (!ctype_alnum($token)) {
    $token = preg_replace('/\D+/', '', $token);
    // $token does now only contain alphanumeric characters
}

Escaping

Escaping is replacing the meta characters of a specific context some string is meant to be placed in. For MySQL queries you should use a function that escapes the meta characters of the context string declaration in MySQL. PHP has the mysql_real_escape_string function for that purpose:

$add = "INSERT INTO ids (ID, token) VALUES ('".mysql_real_escape_string($ID)."', '".mysql_real_escape_string($token)."')";

Answer 3

You can use str_replace to remove spaces. But it's not a good practice. How can URL's be modified so? In normal cases it's unreal. Contrary, you should test all input values from user(ID must be an integer, Token shouldn't contains "'" symbol and other checks). Read about sql-injections.

Answer 4

Your function is vulnerable to SQL injection. You should validate all user-received parameters before using them in an SQL query, and pass any strings through mysql_real_escape_string, because then I could just pass in something like example.php?token='; DROP DATABASE; and royally screw up your application.

In your case, you should make a check that the received parameters are in the form that you expect first, return an error to the user if they don't, and only then pass them into the SQL query.

function add_ID($ID, $token)  {
  $id = mysql_real_escape_string($id);
  $token = mysql_real_escape_string($token);

  $add = "INSERT INTO ids (ID, token) VALUES ('$ID', '$token')";
  mysql_query($add);
  echo 'added successfully';
}  

if(isset($_GET['addDeviceID'])) {
  $id    = isset($_GET['id']) ? $_GET['id'] : 0; // in case no ID has been passed in
  $token = isset($_GET['token']) ? $_GET['token'] : '';

  if (!is_numeric($id) {
    die('ID is not a number');
  } 

  // validate token here as well

  add_ID($id, $token);
}

You should also look into parametrized queries, which are an overall much better way of doing SQL queries with parameters than just using string concatenation. For that, look into using the mysqli extension instead of mysql, or at a higher level, PDO.

Answer 5

Your code is assuming the query successfully completes without ever checking if there was an error. I'm guessing it'll be a syntax error due to the spaces. If your ID field is an integer type, then doing ID=123 123 will be the syntax error. Including all the SQL injection and data sanitizing advice in the other answers, you should rewrite your add_ID function as follows:

function add_ID($ID, $token) {
  $query = 'blah blah blah';
  mysql_query($query);
  if (mysql_error()) {
       echo 'ruhroh, someone set us up the bomb: ', mysql_error();
  } else {
       echo 'woohoo, it worked!';
  }
}

At least this will tell you if the query REALLY did succeed, and what blew up if it didn't. Never assume that a database query of any sort will succeed. There's far too many ways for it to blow up (server died, transaction deadlock, connection pool exhausted, out of disk space, etc...) to NOT have even some simplistic error handling as above.