CODEWITHSUNDEEP
phpmysql
ktm
ktm

Reputation: 6085

how to check sql injection in this php code?

how do i check if this is vulnurable ?

function clean($str){
    return mysql_real_escape_string(stripcslashes(strip_tags(htmlspecialchars($str))));
}


if($_POST['submit']) {
    $sql = "INSERT INTO comments(blog_id, dateposted, name, comment) VALUES
                (" . clean($validentry) . "
                , NOW()
                , '" . clean($_POST['name']) . "'
                , '" . clean($_POST['comment']) . "'
                );";
mysql_query($sql);

Upvotes: 1

Views: 709

Answers (3)

Mr.DOC
Mr.DOC

Reputation: 11

use these function for filtering query :
for example : this query ,you can not filtering if you not use the function below

http://www.example123.com/media.php?f=-1+OR+17-7%3d10

Parameter Type Value

f GET -1 OR 17-7=10

function clean($str){
    if(ereg('^[A-Za-z0-9]+$',$str)){
        $str=mysql_real_escape_string($str);
        return($str);
    }else{

       exit();
    }
}

Upvotes: 1

Alix Axel
Alix Axel

Reputation: 154533

Just out of curiosity, did you code that function yourself? I'm asking because I keep seeing clean() and makeAllSecure() functions, each one worst than the previous.

To secure against SQL injections all you need to use is mysql_real_escape_string(), or similars.

Your stripcslashes() is also unnecessary, you might wanna call stripslashes() instead iff (as in if and only if) magic_quotes is On and $str is a user supplied variable, if the iff yields true this should only occur once, normally at the start of your script.

Regarding strip_tags(htmlspecialchars($str)) - it only has the effect of converting ', ", < and > to their HTML entities notation, no tags will be stripped... If you really also want to strip tags what you are looking for is the following:

htmlspecialchars(strip_tags($str))

But this kind of sanitization should occur when you output HTML contents, not when you save in the DB.

Upvotes: 1

Pekka
Pekka

Reputation: 449395

You need to check the blog_id using is_int() - because it is not wrapped in quotes, mysql_real_escape_string() won't be able to sanitize it. That is clearly a vulnerability.

Other than that, your code looks safe but it does a number of unnecessary sanitations.

mysql_real_escape_string() is completely enough for the name and comment fields.

The other calls are not necessary in that constellation; either strip_tags or htmlspecialchars() will do to protect against XSS attacks. However, it is regarded better practice to store the unsafe, raw HTML in the database, and sanitize it when outputting.

The stripcslashes call you can get rid of completely.

Upvotes: 5

Related Questions