0

i had a sql injection in my code, i checked it with havji.

so i was thinking of a fix, and i set the type of my values to integers or strings like this. would this do the trick ? like no more injections ??? NOTHING TO WORRY ABOUT ? my code IS below, sorry for my bad english!

function get_page_by_id($page_id) {
    settype($page_id, "integer");

    global $connection;
    $query = "SELECT * ";
    $query .= "FROM pages ";
    $query .= "WHERE id=" . $page_id ." ";
    $query .= "LIMIT 1";
    $result_set = mysql_query($query, $connection);
    confirm_query($result_set);
    // REMEMBER:
    // if no rows are returned, fetch_array will return false
    if ($page = mysql_fetch_array($result_set)) {
        return $page;
    } else {
        return NULL;
    }
}

CODE BEFORE FIX

function get_page_by_id($page_id) {

    global $connection;
    $query = "SELECT * ";
    $query .= "FROM pages ";
    $query .= "WHERE id=" . $page_id ." ";
    $query .= "LIMIT 1";
    $result_set = mysql_query($query, $connection);
    confirm_query($result_set);
    // REMEMBER:
    // if no rows are returned, fetch_array will return false
    if ($page = mysql_fetch_array($result_set)) {
        return $page;
    } else {
        return NULL;
    }
}
Zac
  • 2,180
  • 2
  • 23
  • 36
Ahmad
  • 13
  • 5
  • Apart from what the others have answered, you might also want to look into [Mysqli](http://php.net/manual/en/book.mysqli.php) (i for improved). – ACJ Jun 08 '13 at 14:17
  • im still a beginner :) – Ahmad Jun 08 '13 at 14:20
  • Being a beginner, you have to read answers throughly and choose them wisely. Otherwise you'll end up with unpleasant consequences. Like in this very case, as for some reason you accepted the only answer that doesn't actually protect you from injection at all. – Your Common Sense Jun 08 '13 at 14:36

2 Answers2

0

What you normally want to do is use a prepared statement instead. For php you can read about one option here.

Here is the example from that page

<?php
$stmt = $dbh->prepare("INSERT INTO REGISTRY (name, value) VALUES (:name, :value)");
$stmt->bindParam(':name', $name);
$stmt->bindParam(':value', $value);

// insert one row
$name = 'one';
$value = 1;
$stmt->execute();

// insert another row with different values
$name = 'two';
$value = 2;
$stmt->execute();
?>
Andreas Wederbrand
  • 38,065
  • 11
  • 68
  • 78
  • i will try to learn more about the prepared statments! thanks for the answer. but does setting the type fix my problem for now ? since im a beginner – Ahmad Jun 08 '13 at 13:43
-1

Try the following code:

$page_id = mysql_real_escape_string(stripslashes($page_id));
MrByte
  • 1,083
  • 2
  • 11
  • 21