in reply to Re^3: DBI do() SQL injection
in thread DBI do() SQL injection

doh...thanks! In all the other occasions like
$dbh->do(qq{ insert into customers (id,name) values('$id','$name')} )
can you get injected?

Replies are listed 'Best First'.
Re^5: DBI do() SQL injection
by choroba (Cardinal) on Oct 19, 2023 at 13:19 UTC
    If you have no control over the contents of $id, you shouldn't use it directly. What if $id was set to 42', 'Batman'); drop table customers; --?

    In fact, I tried it with two different database drivers. Interestingly, in DBD::SQLite, the table wasn't removed, as it doesn't support multiple statements at a time. On the other hand, DBD::Pg happily removed the table and crashed when I tried using it later.

    You can always fix that by wrapping the value into a quote:

    $dbh->do(join "", 'INSERT INTO customers(id,name) VALUES(', $dbh->quote($id), ', ', $dbh->quote($name), ')');

    map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]
      If you have no control over the contents of $id, you shouldn't use it directly.

      Even if you have control, you should use placeholders. Keeping the query string constant and only varying the parameters allows everything below DBI (i.e. the DBD and especially the database engine) to cache the already parsed query string, including all optimizations. Changing the query string for every new value of $id prevents that.

      Alexander

      --
      Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)
      You can always fix that by wrapping the value into a quote
      And is there any advantage to using quote instead of placeholders?
      sub run_do_with_placeholders($dbh, $id, $name) { $dbh->do(qq{ INSERT INTO customers (id,name) VALUES(?, ?)}, undef, $id, $name); }
      There are a few places (table and column names, mostly) where you're required to use quote instead of placeholders, but, IMO, you should always use placeholders for data values where possible and never rely on quoting unless you absolutely have to.
        > There are a few places (table and column names, mostly) where you're required to use quote instead of placeholders

        No, please don't. Use quote_identifier for table and column names.

        quote is useful when placeholders can't be used, e.g. you're sending the SQL statement to a function that doesn't accept any other arguments and you can't change it; but generally placeholders are easier and cleaner.

        map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]

      Even if the table isn't dropped, it's still an injection bug since the code did not behave as desired (i.e. did not use  42', 'Batman'); drop table customers; -- as the id).

      How does dbh quote save the day? What is different from simple quotinq with '' or q or qq ?
        #!/usr/bin/perl use warnings; use strict; use feature qw{ say }; use DBI; my $dbh = 'DBI'->connect('dbi:SQLite:dbname=:memory:', "", ""); my $id = q{42', 'Batman'); DROP TABLE customers; -- }; say qq('$id'); say $dbh->quote($id); __END__ Output: '42', 'Batman'); DROP TABLE customers; -- ' '42'', ''Batman''); DROP TABLE customers; -- '

        map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]
Re^5: DBI do() SQL injection
by hippo (Archbishop) on Oct 19, 2023 at 13:05 UTC

    Yes, so don't do that! :-)


    🦛