0

I have simple jTable jQuery webpage. Since I didn't recieve much response in my previous topic: Safe MySQL password on shared hosting, I've started to research on my own. I'm using PHP based script for MySQL connection and for Pagination. From what I've researched, queries with no input data from user can't be harmed by hacker unless the PHP file is directly accessed and/or replaced. Therefore I decided to put it above my web root. I've added pagination feature for jTable, which uses SQL query with some input data from JS script. For heaven's sake I decided to protect this query from any malicious SQL Injections.

The query:

"SELECT * FROM people LIMIT " . $_GET["jtStartIndex"] . "," . $_GET["jtPageSize"] . ";"

What I did is I've added casts to int, for ex:

"SELECT * FROM people LIMIT " . (int)$_GET["jtStartIndex"] . "," . (int)$_GET["jtPageSize"] . ";"

Is it safe enough? As far as I remember, any string that will go there will be parsed to 0 by (int) cast.

Community
  • 1
  • 1
mwilczynski
  • 3,062
  • 1
  • 17
  • 27
  • 2
    Use mysqli or pdo with prepared statements to begin with – Andy Holmes Feb 14 '14 at 10:17
  • no, it's not safe. `order by 7` and whatnot makes very little sense. – Marc B Feb 14 '14 at 10:18
  • Wouldn't cast to int from `order by 7` give 0 as we have letter first? – mwilczynski Feb 14 '14 at 10:24
  • My bad, I've gave the example from jTable tutorial where sorting is used. Here I don't want to used sorting so I only want to secure data that is used in LIMIT. It's edited now. Note that query is an example only from jTable tutorials. – mwilczynski Feb 14 '14 at 10:31
  • 1
    Why try to find alternative ways to [prevent SQL injection](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1)? This is a solved problem. – deceze Feb 14 '14 at 10:32
  • @deceze - Please correct me if I'm wrong: Am I doing the same as I would do with for example mysqli code: `$stmt->bind_param('i', $startIndex);` ? – mwilczynski Feb 14 '14 at 10:41
  • 1
    Yes, generally you'll get the same result. However, separating the query and binding the parameters is inherently safer. I tiny typo may subvert your manual casting entirely without you noticing. That's a lot harder with bound values. – deceze Feb 14 '14 at 10:55

1 Answers1

1

Some people think that prepared statement is the only way to be safe about sql injection, it is not true, doing a manual filter is also effective (but verbose).

In this case is fine the solution but it could be wrapper (for example for capturing errors)

<?php
// this function is not needing unless it is used recurrently.
function safe_get_int($value) { 
     return intval(@$_GET[$value]); 
}
$init= safe_get_int("jtStartIndex") ;
$page= safe_get_int("jtPageSize") ;
if (($init>0 && $init<=9999) && ($page>0 && $page<100)) {
   $sql = "SELECT * FROM people LIMIT {$init},{$page};";
} else {
   $sql = "SELECT * FROM people LIMIT 0,20;"; // standard value
}
?>

results:

entering a number = return the number.

entering a text = return zero.

entering a special character = return zero.

missing the value = return zero. (it is why i added the @).

entering a negative = it is considered as a zero value.

(added), entering a big number is also restricted. For example, a big page size could crash some server.

magallanes
  • 6,583
  • 4
  • 54
  • 55