SQL Injection with added magic_quotes assistance (the joys of legacy code maintenance)

 

Sometimes you really have to laugh (or shoot yourself) when you come across legacy code / the mess some other developer(s) left behind. (Names slightly changed to protect the innocent)

class RocketShip {

    function rahrah() {
        $sql = "insert into foo (rah,rahrah,...) 
            values ( '" . $this->escape_str($this->meh) . "', ...... )";
        mysqli_query($this->db_link, $sql) or 
            die("ERROR: " . mysqli_error($this->db_link));
        $this->id = mysqli_insert_id($this->db_link);
    }

    function escape_str($str)
    {
        if(get_magic_quotes_gpc())
           { $str = stripslashes($str);}
        //echo $str;
        //$clean = mysqli_real_escape_string($this->db_link,$str);
        //echo $clean;
       return $str;
    }
// ....
    function something_else() {
         mysqli_query($this->db_link, 
            sprintf("insert into fish(field1,field2) values('%s', '%s')", 
            $this->escape_str($this->field1), 
            $this->escape_str($this->field2));

    }
}

You’ve got to just love the :

1. Lack of Error handling / logging.
2. Functionality of the escape_str function which is only making matters worse (and could never have worked due to the variable names)
3. Use of sprintf  and %s ….(obviously %d could be useful)
4. Documentation?

Dare I uncomment the mysqi_real_escape_string and fix escape_str’s behaviour?

In other news, see this tweet – 84% of web apps are insecure; that’s a bit damning. But perhaps not surprising given code has a far longer lifespan than you expect….