2

I have a url, that when valid would look like this:

site.com/page.php?id=12345

I'm trying to understand if we're vunderable to sql injection. In this particular instance, the value should only be a positive integer value, since it's an id number. We do sometimes use other variables that could be a letter, or a string of text, for example, the search results pages.

An example of the code used to extract the ID variable is here:

$variable = "0";
if (isset($HTTP_GET_VARS["id"])) {
  $variable = (get_magic_quotes_gpc()) ? $HTTP_GET_VARS["id"] : addslashes($HTTP_GET_VARS["id"]);
}

In most instances of getting a variable from the url, it is approached this way.

Is this doing anything to prevent sql injections?

Should I be using mysql_real_escape_string?

I've been reading about prepared statements, but it seems daunting and we use these variables all over the place on site with a lot of pages and queries. Going through and replacing them just isn't viable in the short or mid term.

If there was an alternative way to go about validating the data without prepared statements, any advice would be much appreciated.

Thanks in advance.

Kevin
  • 1,685
  • 7
  • 28
  • 55
  • Which PHP version are you using? – BlitZ May 15 '13 at 05:52
  • You should be using `mysqli` or `pdo` instead of the old and soon to be deprecated `mysql` functions. – Cyclonecode May 15 '13 at 05:54
  • 1
    use prepared statements check this http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php/14110189#14110189 – NullPoiиteя May 15 '13 at 05:55
  • `Going through and replacing them just isn't viable in the short or mid term.` then be prepare for injections. `addslashes` is very easy to byepass. – itachi May 15 '13 at 05:56
  • If prepared statemets is not an option, sanitizing input with typecast + `mysql_real_escape_string` is only option. But in the end, better to rewrite it. – BlitZ May 15 '13 at 05:57
  • Since your id is numerical you can cast it as integer first. $id = (int) $_GET['id']; then pass it to mysql_real_escape_string() – ChamingaD May 15 '13 at 06:07

1 Answers1

4

Is this doing anything to prevent sql injections?

No.

Should I be using mysql_real_escape_string?

No.

If there was an alternative way.

No.
Every way will require rewriting of all the code - this way or another.

However, the choice is yours.
If site's value doesn't worth efforts required to rewrite it properly - well, keep it as is.
If the value is high - try to hire someone to do the job for example.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345