http://qs321.pair.com?node_id=11149720

*alexandre* has asked for the wisdom of the Perl Monks concerning the following question:

Hi, I've the following code to start to paginate a search and want to not allow SQL injection
sub doSearch { my $article = $query->param ('search_name'); my $category = $query->param("categories"); my $string = ""; my $add; my $select = "DISTINCT article.nom"; my $from ; my $where; my $test = $SERVER{'all_categories'}; $category = trimwhitespace($category); $test = trimwhitespace($test); if ($category ne $test) { $add .= "AND article.ref_categorie = categorie_libelle_langue.ref +_categorie AND libelle.libelle = '$category' AND categorie_libelle_la +ngue.ref_libelle = libelle.id_libelle AND categorie_libelle_langue.re +f_langue = langue.id_langue AND langue.key = '$lang'"; $from .= "article,categorie_libelle_langue, libelle, langue"; }else { #$category = ''; $from .= "article"; } my $total = '0'; my $username = $query->param("username"); my ($cat)= $db->sqlSelect("ref_categorie", "categorie_libelle_lan +gue, libelle, langue", "libelle.libelle = '$category' AND categorie_l +ibelle_langue.ref_libelle = libelle.id_libelle AND categorie_libelle_ +langue.ref_langue = langue.id_langue AND langue.key = '$lang'"); my ($userID)= $db->sqlSelect("id_personne", "personne", "nom_util +isateur = '$username'"); my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst)=localtim +e(); my $date = sprintf "%4d-%02d-%02d",$year+1900,$mon+1,$mday; my ($d)= $db->sqlSelect("count(article.nom)", $from, "article.nom + like '%$article%' AND ref_statut = '3' $add"); my $total = $d;
I want to know how is it safe to do sqlSelect queries within this code
</sub sqlSelect { my $from = shift || ''; my $select = shift || ''; my $where = shift || ''; my $other = shift || ''; my $other2 = shift || ''; my $sql="SELECT $select "; $sql.="FROM $where "; $sql.="WHERE $other "; my ($c)=$dbh->prepare($sql) or die "Sql has gone to hell\n"; #print "Content-Type: text/html\n\n"; #print "SQL : $sql \n"; if(not ($c->execute())) { my $err=$dbh->errstr; return undef; } my (@r)=$c->fetchrow(); $c->finish(); return @r; }
Thanks for your advice

Replies are listed 'Best First'.
Re: Avoid SQL injection
by Corion (Patriarch) on Jan 20, 2023 at 11:49 UTC

    Avoiding SQL injection is simple. Use DBI placeholders.

    Instead of concatenating your values into your string, put ? there, and then place the values in ->execute(...).

    You have structured your code around concatenating strings, so I would first replace the sqlSelect</p> with something like:

    sub sqlSingleRow( $sql, @placeholders ) { my $sth = $dbh->prepare( $sql ); $sth->execute(@placeholders); # Do you really get only ever a single row back? my (@r)=$c->fetchrow(); $c->finish(); return @r; }

    ... and then use that routine like:

    my $sql_cat = <<'SQL'; select ref_categorie from categorie_libelle_langue , libelle , langue from ref_categorie where libelle.libelle = ? AND categorie_libelle_langue.ref_libelle = libelle.id_libel +le AND categorie_libelle_langue.ref_langue = langue.id_langue +AND langue.key = ? SQL my ($cat) = sqlSingleRow($sql_cat, $category, $lang );
Re: Avoid SQL injection
by hippo (Bishop) on Jan 20, 2023 at 11:49 UTC

    You will want to refactor this code to use place holders instead. See for example the section on place holders in Databases made easy.


    🦛

Re: Avoid SQL injection
by marto (Cardinal) on Jan 20, 2023 at 11:55 UTC
    my $from = shift || ''; my $select = shift || ''; my $where = shift || ''; .... $sql.="FROM $where "; $sql.="WHERE $other ";

    Are you sure this is your code? It reads as though you are creating sql with a from containing the where clause. Regardless, it's messy. Bobby-tables.com has hints on placeholders/bind variables and the use of quote_identifier for identifiers etc. As a side note, what do you have against printing $!? previously among others.