3

This is probably a repeated question but I really can't find any answers. I have a forgotten password script which grabs the users email address from the URL using $_GET. The script then checks the $_GET for error messages then passes through to mysql to check weather the user id is the same.

Now the script itself is fine but the user id comes back as a string instead of an integer in var_dump(). I was going to let it go but then I read that if the user id is not an integer in a $_GET variable it could lead to an attack, which has got me a little bit worried. I have tried to change the num_rows value to an int but with no success, because num_rows returns an array. The code that checks the user id is:

if (mysql_num_rows($result) == 1) {
    // NEED TO TURN THIS ARRAY TO AN INT
     list($userId) = mysql_fetch_array($result, MYSQL_NUM);  
}

//elseif (mysql_num_rows($result) <= 0) {
else {
    $wrong = '<p style="color: red">Something went wrong. Please try again</p>';
 }

Like I said before the script itself is fine, it's just that I read something and now can't get it out of my head, which has lead me to ask you guys. Doe's a user id have to be an integer or can i get away with it as a string?

EDIT: Thanks for all your comments and suggestions. There's still a lot to learn in php. Cheers again.

bluefiredragon
  • 55
  • 1
  • 5
  • If your script expects an integer, you should validate that the input is an integer. Values in `$_GET` will be strings, but you can check them using `is_numeric` and convert them using `(int)`. – George Cummins May 28 '13 at 15:07
  • The thing you grab from the URL should be an int. If it is a string, that is a vulnerability. When you retrieve the ID from the Database the value can be a string, as long as you trust the contents of your database. – 11684 May 28 '13 at 15:07
  • 1
    use [**FILTER_VALIDATE_INT**](http://php.net/manual/en/function.filter-var.php) . [_Here is a reason why (int) may not be desirable in this case_](http://stackoverflow.com/questions/4789624/php-security-int-vs-filter-validate-int). and then, [_Here is a further example of it_](http://codepad.org/kZGrkNuG) – itachi May 28 '13 at 15:15
  • Where are you doing the query? Could you share that piece of code with us as well? – Ja͢ck May 28 '13 at 15:16

4 Answers4

3

The user ID doesn't have to be an integer, however it is usually

  • unique (one per user)
  • formatted a way that it is searchable efficiently in a database (for instance)
  • short so that it doesn't take much space, especially for large users bank

therefore, ideally, that would be an integer (which number of bits must cover the largest number of expected users)

  • unique - for instance allocated automatically (incrementally) by a database
  • efficiently indexed, and being the small pointer to a larger user table
  • short

one of the drawbacks is that if the integer is given to the client browser (eg in a form), the client may guess the number of user / change the form user ID since numbers in sequence can be easily guessed... (that needs to be prevented via further security)

Déjà vu
  • 28,223
  • 6
  • 72
  • 100
1

Everything you pull from the $_GET array is going to be a string, owing to the fact that there's no type information sent through http.

You should, however, always be sanitizing user input or using prepared statements to prevent SQL injection attacks.

Sam Dufel
  • 17,560
  • 3
  • 48
  • 51
1

You mixed things up, I'll try to clarify them. When PHP receives data via HTTP protocol, be it GET or POST - all the data is considered to be a string because there is no way to safely identify what the data should be.

That means you can try to coerce a piece of data you got via $_GET or $_POST into a different type internally, using typecasting, such as $id = (int)$_GET['id'];

As for databases, the id (let's call it Primary Key) should always be an integer.

Why: - InnoDB uses a certain principle internally when it comes to actual data-structure organisation and writing the information to the disk. It accomplishes a significant performance gain by using sequentially incremented integers - It's easy to create unique identifiers using integers. Just increment some number by an offset every time you do an insert and you get a unique number assigned to the row, no need for a complex algorithm that calculates unique identifier such as UUID() etc.

Long story short - keep the user's ID as integer in MySQL and if you receive data via $_POST/$_GET - type cast them into what you want them to be (int, float, string etc.)

N.B.
  • 13,688
  • 3
  • 45
  • 55
0

A user ID can be anything you want it to be ... I would only suggest that it be something that has a unique key on it in the database and doesn't change over time.

ID's are traditionally used because it is good practice to have an auto-incrementing primary key on your tables anyway and it meets all of the criteria to be an effective user id.

Orangepill
  • 24,500
  • 3
  • 42
  • 63
  • 1
    It's not good practice, the reasons are actually well defined. – N.B. May 28 '13 at 15:13
  • 1
    @N.B. Perhaps it would help to share the link where those reasons are stated. – Ja͢ck May 28 '13 at 15:18
  • @Jack - if I had such a link that explains *all* the reasons, I'd just provide it instantly, but unfortunately I'm without one – N.B. May 28 '13 at 15:21
  • @N.B. For this use case I agree that and auto-incrementing ID should be used but there are use cases where adding an ID to a table adds overhead and abstraction but nothing else. – Orangepill May 28 '13 at 15:21
  • There's no such case where adding an auto_increment to a table adds overhead, if we talk about innodb, and that's a fact. It's related to how it works internally and how it organizes its b-tree that holds indexes. If you don't specify one yourself, it will create a hidden one which is a bigint so you'll end up spending 8 bytes instead of usual 4. If you don't specify an integer as PK, but a string, then you're in world of fun when it comes to inserting records and debugging why it's so slow. – N.B. May 28 '13 at 15:24
  • I was talking about query overhead as opposed to insert or storage overhead. For a customer record I am going to put in the state abbreviation and have that validate against a state table that uses a string key as opposed to storing an state_id and forcing a join on the states table every time I want to resolve it to text. – Orangepill May 28 '13 at 15:40