2008/6/12 Michael Peters <[EMAIL PROTECTED]>:
> Brian Gaber wrote:

>> # Determine MySQL locks table name
>> my $sth = $dbh->prepare("SELECT * FROM region_props WHERE region =
>> '$region'");
>> $sth->execute();

> Btw, this is *really* bad security wise. $region is coming straight from the
> browser. You're setting yourself up for an SQL Injection attack. Imagine I
> request some URL like:

>  regDelLocks.pl?region= %27blah%27%3B+DROP+ALL+DATABASES

> Guess what will happen? Preventing this is really easy. Just use SQL bind 
> params.

> my $sth = $dbh->prepare("SELECT * FROM region_props WHERE region = ?");
> $sth->execute($region);

Hear hear!

It can be true that sometimes you can't (or have good reason to not
want to) use placeholders.

For instance,it's feasible to build a totally dynamic query by pushing
whereclauses into an array and values into another array.

It's also feasible to build a variable length IN() list against some
array like, for instance,  "WHERE colname IN(@{[(join ', ', '?' x
scalar @yourarray]}" into the statement (or using a sprintf much the
same way), it often doesn't read well.

In those cases it's understandable that you'd want to build your query
dynamically, but there's no excuse for unsafeness!

Examples:

Placeholders:
my @things_to_check_for = qw(foo bar baz luhrman);
my $q = <<"EOF";
SELECT thingy
   FROM doohickeys
 WHERE type IN(@{[join ', ', ('?') x scalar @things_to_check_for]})
EOF
my $st = $dbh->prepare($q);
$st->execute(@things_to_check_for);

# if the @{[]} boggles you
my @things_to_check_for = qw(foo bar baz luhrman);
my $q = sprintf <<"EOF", join ', ', ('?') x scalar @things_to_check_for;
SELECT thingy
   FROM doohickeys
 WHERE type IN(%s)
EOF
my $st = $dbh->prepare($q);
$st->execute(@things_to_check_for);

# More explictly:
my @things_to_check_for = qw(foo bar baz luhrman);
my $things_to_check_for = join ', ', ('?') x scalar @things_to_check_for;
my $q = sprintf <<"EOF";
SELECT thingy
   FROM doohickeys
 WHERE type IN($things_to_check_for)
EOF
my $st = $dbh->prepare($q);
$st->execute(@things_to_check_for);

This is a rather contrived example using a very simple query however.
Getting more complex than that, or if you're pandering to PHP
programmers (who are used to their crap database interfaces that only
let you use bind variables on input if you intend to use them on
output too, for no apparent reason), you have potential reasons to
directly create your statement (also, it makes it easier to spit out a
runnable query for debugging, because you can just print the very
statement out*).

So use DBI's quote() method:

Safe without placeholders:

my @things_to_check_for = qw(foo bar baz luhrman);
my $q = <<"EOF";
SELECT thingy
   FROM doohickeys
 WHERE type IN(@{[join ', ', map $dbh->quote($_), @things_to_check_for]})
EOF

(if you are using placeholders everywhere, quote can still be useful
for debugging, assuming you are ONLY using placeholders or at the very
least not writing any statements with a literal question mark in
them). You can take your statement with placeholders and do this:

my $rows = $st->execute($value1, $value2, $value3);

unless ($rows) {
    my $show_query = $q;
    $show_query =~ s/\?/\%s/g;
    printf <<"EOF", $st->errstr, map $dbh->quote($_), $value1, $value2, $value3;
<div id="oops">
  <div id="oopsException">SQL Statement failed: %s</div>
  <div id="statementDump"><pre>$show_query</pre></div>
</div>
EOF
}

-- 
Dodger

Reply via email to