0

Our Sitelock website security has picked up some pages on our website that are vulnerable to attacks. It shows the description "Injection point: GET; Injection parameter: id; Injection type: numeric" I think the mentioned code is 

$sa1=DBSelect("select * from tbl where id='".$_REQUEST['sid']."'");

How can we fix it? Any idea?

asitha
  • 219
  • 1
  • 4
  • 17

1 Answers1

0

First off, you shouldn't use $_REQUEST as it reads $_GET , $_POST and $_COOKIE variables and looks for the value in all of these.

Secondly, you aren't validating the user input to check whether it's safe. If an ID is an integer you should at least do something like this.

$sid = (int) $_GET['sid'];
$sa1=DBSelect("select * from tbl where id='" . $sid . "'");
Pankucins
  • 1,690
  • 1
  • 16
  • 25
  • That's quite inefficient and limited approach. – Your Common Sense Jun 03 '13 at 05:45
  • I agree, but my goal was to point out that validation and making the input secure before querying the database is the problem. – Pankucins Jun 03 '13 at 05:52
  • YES, it's *input* validation indeed a problem that leads straight to injection. You have to format your queries instead of "validating input" and have to do it much closer to query execution than in your code – Your Common Sense Jun 03 '13 at 05:56
  • @Pankucins - Actually what you have in mind is the right thing. If one expects an integer then only accept integers, otherwise just throw an error. Maybe the function `ctype_digit()` could be a good choice then. Then if you know it is an integer and therefore harmless input, then add it to the query as integer (your example still enquotes it with '...' like a string). Even better would be parametrized queries, because then the database can prevent such mistakes and escapes the input if necessary. – martinstoeckli Jun 03 '13 at 06:48