3

I am currently writing a small script in perl to connect to my db, retrieve some data, and show it to the user. The data retrieved depends of parameters given by the user.

I am using dbh->quote to escape the quotes:

...
my $dbh=DBI->connect(***);

my $myquery="SELECT * FROM customers WHERE clientName =".$dbh->quote(param('name')) . " AND pass =".$dbh->quote(param('pass'));
my $sth=$dbh->prepare($myquery);
$sth->execute();

my $output=$sth->fetch();
if ($output){
    print @$output;
}
...

A friend told me that it might not be secure, and that he read that someone found a vulnerability. I am just beginning with perl, but I would like to understand what is that vulnerability.

After some digging I found this document (pdf) that seems to talk about it, but I am not able to reproduce the bug.

14
  • 4
    whether it is secure or not, it is silly to use it instead of just $myquery='SELECT * FROM customers WHERE clientName=? AND pass=?'; ...; $sth->execute( param('name'), param('pass') );
    – ysth
    Oct 27, 2016 at 0:13
  • 1
    @ysth, I heard you want to use quote for performance reasons, because the DB isn't capable of making some optimizations if it doesn't know the type of the parameters in advance. That said, I heard that some DBD actually replace the parameters with quoted values before passing the query to the DB engine rather than passing the provided query and parameters.
    – ikegami
    Oct 27, 2016 at 2:26
  • 1
    @J.Doe, The only issue I can think of if one of information leak if the query gets logged somewhere. Any insecurities (bugs) in quote should be reported immediately, and will be fixed immediately.
    – ikegami
    Oct 27, 2016 at 2:30
  • 1
    @ikegami the performance thing is almost never an issue for real. It can be with true server-side prepared queries, but e.g. DBD::mysql doesn't use those by default for that reason. In normal situations the performance is the same. Some DBDs do actually just quote in the driver, as you say, and others pass the query to the server as a data structure with SQL and values separate, but the types are known.
    – hobbs
    Oct 27, 2016 at 3:38
  • @ikegami however there is an issue here, which is with param's list-context behavior and quote's optional second arg. I've explained it in my answer.
    – hobbs
    Oct 27, 2016 at 3:39

2 Answers 2

12

The problem mostly doesn't lie with quote per se. quote is secure if used properly (although it's not the best choice in this situation). However, if param is the param from CGI.pm, or from anything else that has similar behavior, you have a big problem.

You see, param is context-sensitive. In scalar context, if the parameter has a single value (name=foo), it returns that value, and if the parameter has multiple values (name=foo&name=bar) it returns an arrayref. In list context, it returns a list of values, whether there are zero, one, or many. The argument list of a method (such as quote) is a list context. That means that someone using your app can cause quote to receive two values, and quote's optional second argument is an SQL data type that the first argument should be treated as. If the data type is a non-string type like NUMERIC, then quote will pass its first argument through without any quoting. This constitutes an opportunity for SQL injection.

Recommendations:

  1. Although quote is safe when used properly, placeholders are better, safer, and harder to use wrong. Use DBI placeholders whenever possible, instead of quote.

  2. Don't use CGI's param in argument lists, hash constructors, or any other place where it could return an unexpected number of items and ruin your day. Either put scalar out front, assign to a scalar, or assign to an array. Or, better yet, avoid CGI.pm and workalike interfaces entirely.

  3. Don't store passwords as plaintext in the database. If anyone ever does get access to parts of your database, your users' passwords will be exposed to them. Passwords should be hashed and there are good, easy-to-use Perl modules for doing so (Authen::Passphrase comes to mind).

  4. Don't pass passwords as URL parameters. URLs are easily leaked through HTTP referers, browser history, careless copy/paste, etc. Passwords should be POSTed in forms, preferably over a secure connection.

1
  • 1
    Ah cool! I think newer versions of param warn when you use it in list context to avoid surprises like this
    – ikegami
    Oct 27, 2016 at 3:43
1

You may want to reach out to dbi-users mailing list to ask for what vulnerabilities exist. Adding a quote doesn't making things bad as long as you are escaping them. You still make sure you are aware of all SQL injections attacks. You may also want to avoid passing your password as a url GET parameter (prefer POST requests), since urls (along with any params) are not encrypted and can be easily read.

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.