Bug#622919: Raise severity?
gregor herrmann
gregoa at debian.org
Tue Jan 3 21:59:55 UTC 2012
On Tue, 03 Jan 2012 21:35:07 +0100, gregor herrmann wrote:
> > Looking at the diff again (attached for reference), it's quite long
> > and also includes documentation fixes.
> > I guess we have to look a bit to trim it down to the relevant parts.
> Quick attempt (I looked at the diff in upstream 0.67 -> 0.68 and
> ripped out the parts from the original patch that had no equivalent
> in the upstream diff).
Hm, so I also forgot the second attachment? Oh my.
Cheers,
gregor
--
.''`. Homepage: http://info.comodo.priv.at/ - OpenPGP key ID: 0x8649AA06
: :' : Debian GNU/Linux user, admin, & developer - http://www.debian.org/
`. `' Member of VIBE!AT & SPI, fellow of Free Software Foundation Europe
`- NP: Steppenwolf: It's Never Too Late
-------------- next part --------------
diff -u libjifty-dbi-perl-0.60/debian/changelog libjifty-dbi-perl-0.60/debian/changelog
--- libjifty-dbi-perl-0.60/debian/changelog
+++ libjifty-dbi-perl-0.60/debian/changelog
@@ -1,3 +1,11 @@
+libjifty-dbi-perl (0.60-1+squeeze1) UNRELEASED; urgency=high
+
+ * Team upload.
+ * [SECURITY] Apply patch prepared by upstream that backports fixes for SQL
+ injection weaknesses (closes: #622919).
+
+ -- gregor herrmann <gregoa at debian.org> Tue, 19 Apr 2011 23:53:52 +0200
+
libjifty-dbi-perl (0.60-1) unstable; urgency=low
[ Jonathan Yu ]
only in patch2:
unchanged:
--- libjifty-dbi-perl-0.60.orig/lib/Jifty/DBI/Collection.pm
+++ libjifty-dbi-perl-0.60/lib/Jifty/DBI/Collection.pm
@@ -1201,16 +1254,9 @@
# }}}
- # Set this to the name of the column and the alias, unless we've been
- # handed a subclause name
-
- my $qualified_column
- = $args{'alias'}
- ? $args{'alias'} . "." . $args{'column'}
- : $args{'column'};
- my $clause_id = $args{'subclause'} || $qualified_column;
-
- # XXX: when is column_obj undefined?
+ # $column_obj is undefined when the table2 argument to the join is a table
+ # name and not a collection model class. In that case, the class key
+ # doesn't exist for the join.
my $class
= $self->{joins}{ $args{alias} }
&& $self->{joins}{ $args{alias} }{class}
@@ -1222,7 +1268,44 @@
$self->new_item->_apply_input_filters(
column => $column_obj,
value_ref => \$args{'value'},
- ) if $column_obj && $column_obj->encode_on_select;
+ ) if $column_obj && $column_obj->encode_on_select && $args{operator} !~ /IS/;
+
+ # Ensure that the column has nothing fishy going on. We can't
+ # simply check $column_obj's truth because joins mostly join by
+ # table name, not class, and we don't track table_name -> class.
+ if ($args{column} =~ /\W/) {
+ warn "Possible SQL injection on column '$args{column}' in limit at @{[join(',',(caller)[1,2])]}\n";
+ %args = (
+ %args,
+ column => 'id',
+ operator => '<',
+ value => 0,
+ );
+ }
+ if ($args{operator} !~ /^(=|<|>|!=|<>|<=|>=
+ |(NOT\s*)?LIKE
+ |(NOT\s*)?(STARTS|ENDS)_?WITH
+ |(NOT\s*)?MATCHES
+ |IS(\s*NOT)?
+ |IN)$/ix) {
+ warn "Unknown operator '$args{operator}' in limit at @{[join(',',(caller)[1,2])]}\n";
+ %args = (
+ %args,
+ column => 'id',
+ operator => '<',
+ value => 0,
+ );
+ }
+
+
+ # Set this to the name of the column and the alias, unless we've been
+ # handed a subclause name
+ my $qualified_column
+ = $args{'alias'}
+ ? $args{'alias'} . "." . $args{'column'}
+ : $args{'column'};
+ my $clause_id = $args{'subclause'} || $qualified_column;
+
# make passing in an object DTRT
my $value_ref = ref( $args{value} );
@@ -1248,27 +1337,28 @@
#since we're changing the search criteria, we need to redo the search
$self->redo_search();
- if ( $args{'column'} ) {
-
- #If it's a like, we supply the %s around the search term
- if ( $args{'operator'} =~ /MATCHES/i ) {
- $args{'value'} = "%" . $args{'value'} . "%";
- } elsif ( $args{'operator'} =~ /STARTS_?WITH/i ) {
- $args{'value'} = $args{'value'} . "%";
- } elsif ( $args{'operator'} =~ /ENDS_?WITH/i ) {
- $args{'value'} = "%" . $args{'value'};
- }
- $args{'operator'} =~ s/(?:MATCHES|ENDS_?WITH|STARTS_?WITH)/LIKE/i;
-
- #if we're explicitly told not to to quote the value or
- # we're doing an IS or IS NOT (null), don't quote the operator.
-
- if ( $args{'quote_value'} && $args{'operator'} !~ /IS/i ) {
- if ( $value_ref eq 'ARRAY' ) {
- map { $_ = $self->_handle->quote_value($_) } @{ $args{'value'} };
- } else {
- $args{'value'} = $self->_handle->quote_value( $args{'value'} );
- }
+ #If it's a like, we supply the %s around the search term
+ if ( $args{'operator'} =~ /MATCHES/i ) {
+ $args{'value'} = "%" . $args{'value'} . "%";
+ } elsif ( $args{'operator'} =~ /STARTS_?WITH/i ) {
+ $args{'value'} = $args{'value'} . "%";
+ } elsif ( $args{'operator'} =~ /ENDS_?WITH/i ) {
+ $args{'value'} = "%" . $args{'value'};
+ }
+ $args{'operator'} =~ s/(?:MATCHES|ENDS_?WITH|STARTS_?WITH)/LIKE/i;
+
+ # Force the value to NULL (non-quoted) if the operator is IS.
+ if ($args{'operator'} =~ /^IS(\s*NOT)?$/i) {
+ $args{'quote_value'} = 0;
+ $args{'value'} = 'NULL';
+ }
+
+ # Quote the value
+ if ( $args{'quote_value'} ) {
+ if ( $value_ref eq 'ARRAY' ) {
+ map { $_ = $self->_handle->quote_value($_) } @{ $args{'value'} };
+ } else {
+ $args{'value'} = $self->_handle->quote_value( $args{'value'} );
}
}
@@ -1595,7 +1689,7 @@
$rowhash{'order'} = "ASC";
}
- if ( $rowhash{'function'} ) {
+ if ( $rowhash{'function'} and not defined $rowhash{'column'} ) {
$clause .= ( $clause ? ", " : " " );
$clause .= $rowhash{'function'} . ' ';
$clause .= $rowhash{'order'};
@@ -1603,11 +1697,17 @@
} elsif ( ( defined $rowhash{'alias'} )
and ( $rowhash{'column'} ) )
{
+ if ($rowhash{'column'} =~ /\W/) {
+ warn "Possible SQL injection in column '$rowhash{column}' in order_by\n";
+ next;
+ }
$clause .= ( $clause ? ", " : " " );
+ $clause .= $rowhash{'function'} . "(" if $rowhash{'function'};
$clause .= $rowhash{'alias'} . "." if $rowhash{'alias'};
- $clause .= $rowhash{'column'} . " ";
- $clause .= $rowhash{'order'};
+ $clause .= $rowhash{'column'};
+ $clause .= ")" if $rowhash{'function'};
+ $clause .= " " . $rowhash{'order'};
}
}
$clause = " ORDER BY$clause " if $clause;
@@ -1685,6 +1785,10 @@
} elsif ( ( $rowhash{'alias'} )
and ( $rowhash{'column'} ) )
{
+ if ($rowhash{'column'} =~ /\W/) {
+ warn "Possible SQL injection in column '$rowhash{column}' in group_by\n";
+ next;
+ }
$clause .= ( $clause ? ", " : " " );
$clause .= $rowhash{'alias'} . ".";
only in patch2:
unchanged:
--- libjifty-dbi-perl-0.60.orig/lib/Jifty/DBI/Handle/Pg.pm
+++ libjifty-dbi-perl-0.60/lib/Jifty/DBI/Handle/Pg.pm
@@ -210,12 +210,15 @@
map {
my $alias = $_->{alias} || '';
my $column = $_->{column};
+ if ($column =~ /\W/) {
+ warn "Possible SQL injection in column '$column' in order_by\n";
+ next;
+ }
$alias .= '.' if $alias;
- #warn "alias $alias => column $column\n";
( ( !$alias or $alias eq 'main.' ) and $column eq 'id' )
? $_
- : { %{$_}, alias => '', column => "min($alias$column)" }
+ : { %{$_}, column => undef, function => "min($alias$column)" }
} @{ $collection->{order_by} }
];
my $group = $collection->_group_clause;
only in patch2:
unchanged:
--- libjifty-dbi-perl-0.60.orig/lib/Jifty/DBI/Handle/Oracle.pm
+++ libjifty-dbi-perl-0.60/lib/Jifty/DBI/Handle/Oracle.pm
@@ -251,18 +251,30 @@
= [ @{ $collection->{group_by} || [] }, { column => 'id' } ];
local $collection->{order_by} = [
map {
- ( $_->{alias} and $_->{alias} ne "main" )
- ? { %{$_}, column => "min(" . $_->{column} . ")" }
- : $_
+ my $alias = $_->{alias} || '';
+ my $column = $_->{column};
+ if ($column =~ /\W/) {
+ warn "Possible SQL injection in column '$column' in order_by\n";
+ next;
+ }
+ $alias .= '.' if $alias;
+
+ ( ( !$alias or $alias eq 'main.' ) and $column eq 'id' )
+ ? $_
+ : { %{$_}, column => undef, function => "min($alias$column)" }
} @{ $collection->{order_by} }
];
my $group = $collection->_group_clause;
my $order = $collection->_order_clause;
$$statementref
- = "SELECT main.* FROM ( SELECT main.id FROM $$statementref $group $order ) distinctquery, $table main WHERE (main.id = distinctquery.id)";
+ = "SELECT "
+ . $collection->query_columns
+ . " FROM ( SELECT main.id FROM $$statementref $group $order ) distinctquery, $table main WHERE (main.id = distinctquery.id)";
} else {
$$statementref
- = "SELECT main.* FROM ( SELECT DISTINCT main.id FROM $$statementref ) distinctquery, $table main WHERE (main.id = distinctquery.id) ";
+ = "SELECT "
+ . $collection->query_columns
+ . " FROM ( SELECT DISTINCT main.id FROM $$statementref ) distinctquery, $table main WHERE (main.id = distinctquery.id) ";
$$statementref .= $collection->_group_clause;
$$statementref .= $collection->_order_clause;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-perl-maintainers/attachments/20120103/8d8730dc/attachment-0001.pgp>
More information about the pkg-perl-maintainers
mailing list