1
$query = 'SELECT ROW FROM TABLE LIMIT' . $start . ', ' . $limit;

The $start and $limit are user input, so I think it may be injected by inputting something like: 1;CREATE DATABASE A; for $start, which would create a new database 'A'. What will be the best practice to prevent that? Please be specific, including example code will be nice.

CodTango
  • 345
  • 1
  • 3
  • 15
  • The best bet is to actively validate those values as integers before injecting them – Mark Baker Mar 16 '15 at 19:37
  • 1
    possible duplicate of [How can I prevent SQL-injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jørgen Mar 16 '15 at 19:37
  • This is not a strict duplicate of 'How can I prevent SQL-injection in PHP?' (although I am sure it is a duplicate) as it deals with LIMIT. While that answer should be linked there should be special consideration given to different methods of binding and when such are valid. – user2864740 Mar 16 '15 at 19:42

4 Answers4

1

Request like you mentioned

$start = 1;CREATE DATABASE A;

won't work, as PHP has disabled stacked queries by default.

$start and $limit are numbers, so just say PHP that those vars are integers:

$query = 'SELECT ROW FROM TABLE LIMIT ' . (int)$start . ', ' . (int)$limit;

This will make SQL Injection impossible, but still hackers can break that request and get some info(eg. traces of error, mysql error), so you just need to validate $start and $limit by regex.

Also, prepared statements are the BEST way to prevent and injections.

sota
  • 365
  • 3
  • 8
  • Noting that this is a *special case* for LIMIT, I think this answer is better than pre-checking the values because it ensures *at the SQL site* that injection cannot occur. In all cases where the data does not affect the query shape proper placeholders / parameterized queries should be used. – user2864740 Mar 16 '15 at 19:40
  • @sota 1. It's my mistake, I mean that user input for $start. 2. Is there a doc to address PHP has disabled stacked queries? – CodTango Mar 16 '15 at 20:24
  • @codecoder, yeah, I understood that :) Here's the [doc](http://php.net/manual/en/mysqli.quickstart.multiple-statement.php): "The API functions mysqli_query() and mysqli_real_query() do not set a connection flag necessary for activating multi queries in the server. An extra API call is used for multiple statements to reduce the likeliness of accidental SQL injection attacks. ... If the attacker succeeds in adding SQL to the statement string but mysqli_multi_query is not used, the server will not execute the second, injected and malicious SQL statement." – sota Mar 16 '15 at 20:27
1

Best practice is to use prepared statements with PDO or mysqli, something like

$stmt = $db->prepare("SELECT ROW FROM TABLE LIMIT :limit1, :limit2");

$sth->bindParam(':limit1', $start, PDO::PARAM_INT);
$sth->bindParam(':limit2', $limit, PDO::PARAM_INT);

$stmt->execute();
adeneo
  • 312,895
  • 29
  • 395
  • 388
  • This works with LIMIT? (There are multiple PDO drivers, and I don't think this will work with the one that uses mysql prepared statements - it ought to work with the synthetic one.) – user2864740 Mar 16 '15 at 19:38
  • I'm not certain that LIMIT arguments can be bound – Mark Baker Mar 16 '15 at 19:38
0

You can use a simple check if it is a numeric value using something like:

if (is_numeric($start) && is_numeric($limit)) {
     $query = 'SELECT ROW FROM TABLE LIMIT' . $start . ', ' . $limit;
}
Neo
  • 6,753
  • 1
  • 19
  • 25
  • To be completely safe, using `is_numeric` is probably not enough as it can interpolate hex and other counting system figures, I have seen posts about this on SO although I accept that it is extremely unlikely an `is_numeric` check would allow something that returns a `true` statement, it's not a promised safety check, but it should be further checked to be ***sure*** – Martin Mar 16 '15 at 19:42
  • true, though hex can be a possible numeric value. Can't really see a case where it will cause an injection. Though I would agree typecasting can be done alongside it. – Neo Mar 16 '15 at 19:45
  • As I say I've seen other topics on SO about it, but I will admit I think in general the cases of injection risk with this method are few and far between – Martin Mar 16 '15 at 19:55
0

1) You know start and limit need to be integers so force convert them to be integer types.

2) Use a PDO or MySQLi interface to prevent improper injection of code, DO NOT use native mysql

Martin
  • 22,212
  • 11
  • 70
  • 132