1

I have a php page. It recieves a value for id via get. 2 simple questions:

1 - In my code this is used only once. In an if statement like:

if ($_GET['id']==1){
Things here....
}

That is the only use of this passed id value. Do I need to sanitize it or can I just leave it safely in the if statement without checking it? Can hackers penetrate through this?

2 - Would I need to sanitize it if I had assigned it to a variable like:

$idid=$_GET['id'];
if ($idid==1){
Things here...
}

Like before this is the only use of this variable, it will not be used in echo or mysql etc.

David19801
  • 11,214
  • 25
  • 84
  • 127

4 Answers4

2

You only need to worry about sanitation, if the GET value is inserted in some potentially harmful place, i.e. echo (XSS), mysql_query (SQL Injection), eval (PHP Execution), shell_exec (Shell execution), ... (More extensive list at Exploitable PHP functions)

Just checking for a value doesn't need any sanitation.

Community
  • 1
  • 1
NikiC
  • 100,734
  • 37
  • 191
  • 225
  • Warnings enabled: Not checking for an array's offset existance, will reveal it. --- What ''type of insertion into a potentially harmful place''-category would fall that under for your answer? – hakre Apr 10 '11 at 12:28
  • @hakre: Sorry, I don't quite understand what you are trying to say. – NikiC Apr 10 '11 at 12:29
  • @hakre: Warnings aren't enabled on production servers ;) Furthermore: Why would it be a vulernability, if you would get an undefined offset xyz warning? – NikiC Apr 10 '11 at 12:32
  • @nikic: Information exposure is used in penetration. Also see [Postel's law](http://en.wikipedia.org/wiki/Postel%27s_law). – hakre Apr 11 '11 at 14:16
  • @hakre: From the fact that an `id=abc` GET parameter is found in the URL it may easily be concluded, that this parameter will actually get used. No need to wait for a warning. And furthermore, as I already said, warnings aren't turned on on production servers, *never*. – NikiC Apr 11 '11 at 18:14
  • @nikic: I see no URL in that question and I can only assume that Dave is following the practice you presuppose for production server configuration. – hakre Apr 11 '11 at 19:10
  • @hakre: See, this discussion is pointless, we don't know nothing about what or how he does. Maybe he even has the check for existence. – NikiC Apr 11 '11 at 19:17
  • @nikic: :) -- You made me smile. – hakre Apr 11 '11 at 19:29
1

No you do not need to sanitize it as it does nothing if it no other value than 1.

Allister
  • 851
  • 3
  • 10
  • 21
0

Do I need to sanitize it or can I just leave it safely in the if statement without checking it?

That depends on the level of quality you would like to achieve. It looks like that you access the $_GET array on a key that you don't test for existence.

This can trigger a notice like "Notice: Undefined index: id" which contains the filename as well.

Depending on server configuration, this message might be given to everybody requesting the website in question not providing the id URL query parameter.

Next to information offered to a potential attacker, spreading notices makes it harder for you to achieve a certain level of quality for your overall script, e.g. if you decide to turn notices and errors into feedback for maintaining and improving the script.

Can hackers penetrate through this?

This information can be used to further penetrate your site then.

From the code you've shown you don't make further use of the variable / value and as far as I can say, I don't see an additional issue next to information exposure.

It's always subject what you would like to achieve, security is nothing static. See Defensive programming, Secure input and output handling and Postel's law.

hakre
  • 193,403
  • 52
  • 435
  • 836
-2

use === instead of == to determine the type of the entry. In my opinion you don't need to do hard job to protect the input. Only make sure that the input is really a number.

Nabeel
  • 557
  • 4
  • 15