guillaumeaubert / Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection

PerlCritic policy that attempts to detect the most common sources of SQL injection in manually crafted SQL statements, by detecting the use of variables inside interpolated strings that look like SQL statements.
https://metacpan.org/pod/Perl::Critic::Policy::ValuesAndExpressions::PreventSQLInjection
Other
6 stars 8 forks source link

False positive with operators like `eq`, `&&` #20

Open cashlo opened 6 years ago

cashlo commented 6 years ago

Here's some examples:

my $foo;
my $bar;

"update" eq $foo;
$foo eq "update" && $bar;
$foo ne "select" || $bar;

and the outpout:

SQL injection risk at line 4, column 1. Variables in interpolated SQL string are susceptible to SQL injection: $foo. (Severity: 5) SQL injection risk at line 5, column 9. Variables in interpolated SQL string are susceptible to SQL injection: $bar. (Severity: 5) SQL injection risk at line 6, column 9. Variables in interpolated SQL string are susceptible to SQL injection: $bar. (Severity: 5)

It seems to me some conditions need to be added around here ( https://github.com/guillaumeaubert/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection/blob/master/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm#L398 ) for these operators?

vti commented 6 years ago

Got bitten by this too:

my $branch = $type eq 'delete' ? $change->{old}->{name} : $change->{new}->{name};
tadzik commented 6 years ago

Stumbled upon this too, my testcase:

my $json;
exit if $json->{foo} eq 'delete' and exists $json->{bar};