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 :
- Lack of Error handling / logging.
- Functionality of the escape_str function which is only making matters worse (and could never have worked due to the variable names)
- Use of sprintf and %s ….(obviously %d could be useful)
- 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….