Derek Fountain <dflists@iinet.net.au> writes:
> If another SQL Injection vulnerability turns up (which it might, given the
> state of the website code),
You will never see another SQL injection vulnerability if you simply switch to
always using prepared queries and placeholders. Make it a rule that you
_never_ interpolate variables into the query string. period. No manual quoting
to get right, no subtle security audit necessary: If the SQL query isn't a
constant string you reject it.
Any good driver should support prepared queries. Even on pre-8.0 it should
support them and emulate them by quoting parameters for you. The driver is
much less likely to get this wrong than you are. On 8.0 the driver can pass
the parameters to the server separately from the query in a binary protocol.
The interface you're looking for should look something like:
$sth = $dbh->prepare("select * from foo where id = ?");
$sth->execute($id);
$results = $sth->fetch();
...
Notice that there's absolutely nothing you can do to inject anything using
$id. And there's not really any way to get this code wrong and compromise
security.
Really interpolating variables in the query string should never have been
considered an acceptable coding practice. It's mixing executable code and
program data which has never been a good idea.
There are more complex queries than this where it can be useful to interpolate
some variables, things like
if ($view_all) {
$join_type = "OUTER";
} else {
$join_type = "";
}
$sth = $dbh->execute("SELECT * FROM foo $join_type JOIN bar USING (x) where z = ?");
but these types of queries are relatively rare and careful security analysis
should still be able to show that the query is still completely built up out
of program constants. Not parameters taken from outside data. That's still a
lot easier to prove than checking that every parameter is quoted properly.
--
greg
