Opened 3 years ago

Closed 3 years ago

#1387 closed defect (fixed)

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

Reported by: tome Owned by: bowtie
Priority: minor Milestone:
Component: Debugger for Perl 5 Version: 0.95
Keywords: breakpoints, sql Cc:

Description

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 ) {

Attachments (1)

sql-inject-breakpoints.diff (1.5 KB) - added by tome 3 years ago.
Diff to use parameter markers in Breakpoints.pm

Download all attachments as: .zip

Change History (4)

Changed 3 years ago by tome

Diff to use parameter markers in Breakpoints.pm

comment:1 Changed 3 years ago by tome

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

comment:2 Changed 3 years ago by bowtie

  • Component changed from not classified yet to Debugger for Perl 5
  • Owner set to bowtie
  • Status changed from new to accepted

comment:3 Changed 3 years ago by bowtie

  • Resolution set to fixed
  • Status changed from accepted to closed

thanks tome

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

applied in r18075

Note: See TracTickets for help on using tickets.