PHP MySQL vulnerability

Question

I use this function for preventing SQL injection

  function cleanQuery($string)
{
  if(get_magic_quotes_gpc())  // prevents duplicate backslashes
  {
    $string = stripslashes($string);
  }
  if (phpversion() >= '4.3.0')
  {
    $string = mysql_real_escape_string($string);
  }
  else
  {
    $string = mysql_escape_string($string);
  }
  return htmlentities($string);
}

and i use it like that

$sql = "select * from news where id = ".cleanQuery($id)." ";

the query should be safe when calling the page.php?id=1 however when adding ' to the end of URL like that page.php?id=1' it gives error

Warning: mysql_fetch_object(): supplied argument is not a valid MySQL result resource

that means the page is still having a vulnerability i think, any body has a solution ?

no it means the sql query is invalid, says noting about injection vulnerability — user557846
– user557846, Commented Feb 23, 2011 at 0:07
This query is 100% vulnerable to SQL injection, and judging by this code you haven't a clue what that means. — rook
– rook, Commented Feb 23, 2011 at 22:44
possible duplicate of Warning: mysql_fetch_array(): supplied argument is not a valid MySQL result — Fluffeh
– Fluffeh, Commented Aug 21, 2012 at 11:40

Kenaniah · Accepted Answer · 2011-02-23 00:33:13Z

4

Use PDO's prepared statements. Prepared statements have the nice property of preventing all forms of parameter injection when you parametrize every external variable in your query. That said, be careful of data type conversions.

answered Feb 23, 2011 at 0:33

Kenaniah

5,20126 silver badges28 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

rook Over a year ago

+1 great advice, especially if you don't know what sql injection is.

dobs · Accepted Answer · 2011-02-23 00:11:01Z

2

For integer use (int)

$sql = 'select * from news where id = '.(int) $_GET[id];

answered Feb 23, 2011 at 0:11

dobs

2,7642 gold badges22 silver badges19 bronze badges

Comments

Czechnology · Accepted Answer · 2011-02-23 00:07:47Z

1

If your id is numeric, the easiest solution is simply

$sql = "select * from news where id = '".intval($_GET['id'])."'";

answered Feb 23, 2011 at 0:07

Czechnology

15k10 gold badges66 silver badges88 bronze badges

2 Comments

user629300 Over a year ago

this fixed the problem but is it now safe from mysql injection ??

Czechnology Over a year ago

@user629300, in this one particular case - yes. The user/attacker can't slip anything in the query except an integer and that's what you're expecting.

Pekka · Accepted Answer · 2011-02-23 00:08:47Z

1

The query will break because your condition is not wrapped in quotes. Thus, only numeric values can work.

To make it not break, validate the number before passing the query, or use

$sql = "select * from news where id = '".cleanQuery($id)."' ";

By the way, htmlentities() is unnecessary and potentially harmful in your sanitation function.

answered Feb 23, 2011 at 0:08

Pekka

451k150 gold badges989 silver badges1.1k bronze badges

Comments

Emmanuel · Accepted Answer · 2011-02-23 00:16:42Z

1

If it's an id, just use is_numeric to validate:

if(ctype_digit($_GET['id'])){
    $id = $_GET['id'];
} else {
    $id = 1; // or whatever your default id is
}

Or inline:

$sql = "select * from news where id = '".(ctype_digit($_GET['id']) ? $_GET['id'] : 1)."'";

edited Feb 23, 2011 at 0:16

answered Feb 23, 2011 at 0:08

Emmanuel

5,4835 gold badges53 silver badges74 bronze badges

1 Comment

Dunhamzzz Over a year ago

Better yet use ctype_digit, this will prevent valid numbers like 10e3.

davogotland · Accepted Answer · 2011-02-23 01:44:10Z

prepared statements is always the way to go about sql queries. php has a library called mysqli. the fact that the "i" in mysqli stands for "improved" says alot :)

here's an example! first, i did this to my database:

create database mydatabase default character set = UTF8;
use mydatabase;
create table news(id int auto_increment, title varchar(50), body text, primary key (id));
insert into news(title, body) values('good news','are good');
insert into news(title, body) values('old news','are old');

and then i used this php script (named news.php) to access my table:

<?php
    //my root user doesn't have a password, so third argument is empty string
    $db = new mysqli("localhost", "root", "", "mydatabase");

    if(mysqli_connect_errno()) {
        die("unable to connect to database: " . mysqli_connect_error());
    }

    //change character set to utf8
    if(!$db->set_charset("utf8")) {
        die("Error loading character set utf8:\n{$mysqli->error}");
    }

    //the question marks denote parameters to be bound
    $sql = "SELECT * FROM news WHERE id BETWEEN ? AND ?;";
    $statement = $db->stmt_init();
    $statement->prepare($sql);
    $sqlError = $db->error;

    if($sqlError != "") {
        die("there was a problem with your query<br />\n$sql<br />\nerror reports:<br />\n$sqlError");
    }

    //the "i":s denote both parameters to bind are int
    $statement->bind_param("ii", $min, $max);
    $min = $_GET['min'];
    $max = $_GET['max'];
    $statement->execute();
    $statement->store_result();
    $statement->bind_result($id, $title, $body);

    //everytime fetch is called, a new line is attempted to be read.
    //if a line was read, two things happen:
    //1. true is returned
    //2. the values of the columns in the fetched result row is stored in the
    //      variables bound as results on the line above
    while($statement->fetch()) {
        print "$id, $title, $body";
    }

    $statement->free_result();
    $statement->close();
    $db->close();
?>

i called the script like so:

http://localhost/news.php?min=1&max=2

Collectives™ on Stack Overflow

PHP MySQL vulnerability

6 Answers 6

1 Comment

Comments

2 Comments

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

1 Comment

Comments

2 Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related