Bad Query/Potential SQL injection in Padre/Breakpoints.pm

[tome]

The two methods in Breakpoints.pm interpolate filename (and line number) directly into a mysql query.

This could cause issues if you have a malicious user or a file name with quotes or other SQL-like things.

I think it probably makes sense to use parameter markers. I've attached a diff, but since I'm just coming in from a quick review I didn't want to check in the code unless others had a chance to look at it.

Diff (also attached):

Index: Padre/Breakpoints?.pm
===================================================================
--- Padre/Breakpoints?.pm (revision 18061)
+++ Padre/Breakpoints?.pm (working copy)
@@ -22,15 +22,13 @@

my %bp_action;
$bp_action{line} = $bp_line;

• if ( $#{ $debug_breakpoints->select("WHERE filename = \"$current_file\" AND line_number = \"$bp_line\"") } >= 0 ) {

-
+ if ( $#{ $debug_breakpoints->select("WHERE filename = ? AND line_number = ?", $current_file, $bp_line ) } >= 0 ) {

# say 'delete me';
$editor->MarkerDelete?( $bp_line - 1, Padre::Constant::MARKER_BREAKPOINT() );
$editor->MarkerDelete?( $bp_line - 1, Padre::Constant::MARKER_NOT_BREAKABLE() );

• $debug_breakpoints->delete("WHERE filename = \"$current_file\" AND line_number = \"$bp_line\"");

+ $debug_breakpoints->delete("WHERE filename = ? AND line_number = ?", $current_file, $bp_line);

$bp_action{action} = 'delete';

} else {

-

# say 'create me';
$editor->MarkerAdd?( $bp_line - 1, Padre::Constant::MARKER_BREAKPOINT() );
$debug_breakpoints->create(

@@ -55,8 +53,8 @@

my $editor = Padre::Current->editor;
my $debug_breakpoints = ('Padre::DB::DebugBreakpoints?');
my $current_file = $editor->{Document}->filename;

• my $sql_select = "WHERE BY filename = \"$current_file\" ASC, line_number ASC";
• my @tuples = $debug_breakpoints->select($sql_select);

+ my $sql_select = "WHERE BY filename = ? ASC, line_number ASC";
+ my @tuples = $debug_breakpoints->select($sql_select, $current_file);

for ( 0 .. $#tuples ) {

[tome]

Diff to use parameter markers in Breakpoints.pm

[tome]

Oops, too much mysql on the brain. I meant to write they are interpolated into a SQL query (not mysql query).

[bowtie]

thanks tome

as to why info only ​http://www.upperbound.com/article.php/20050117232424605

applied in r18075