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
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 <gre...@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; }
signature.asc
Description: Digital signature