r72824 - in /trunk/libjifty-dbi-perl: Changes META.yml SIGNATURE debian/changelog lib/Jifty/DBI.pm lib/Jifty/DBI/Collection.pm lib/Jifty/DBI/Handle/Oracle.pm lib/Jifty/DBI/Handle/Pg.pm

gregoa at users.alioth.debian.org gregoa at users.alioth.debian.org
Tue Apr 19 21:40:33 UTC 2011


Author: gregoa
Date: Tue Apr 19 21:39:40 2011
New Revision: 72824

URL: http://svn.debian.org/wsvn/pkg-perl/?sc=1&rev=72824
Log:
* New upstream release:
  - fix SQL injection weaknesses (closes: #622919)

Modified:
    trunk/libjifty-dbi-perl/Changes
    trunk/libjifty-dbi-perl/META.yml
    trunk/libjifty-dbi-perl/SIGNATURE
    trunk/libjifty-dbi-perl/debian/changelog
    trunk/libjifty-dbi-perl/lib/Jifty/DBI.pm
    trunk/libjifty-dbi-perl/lib/Jifty/DBI/Collection.pm
    trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Oracle.pm
    trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Pg.pm

Modified: trunk/libjifty-dbi-perl/Changes
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/Changes?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/Changes (original)
+++ trunk/libjifty-dbi-perl/Changes Tue Apr 19 21:39:40 2011
@@ -1,4 +1,18 @@
 Revision history for Perl extension Jifty::DBI.
+
+0.68 2011-04-14
+- Security:
+    * Prevent SQL injection in column names, operators, order and group by
+      (Alex Vandiver)
+    * Fix distinct_query to catch injection and correctly rewrite to
+      function => '' (Alex Vandiver)
+    * Prevent SQL injection via IS
+
+- Fixes:
+    * There is no need to check $args{column} around our LIKE adjustments
+      (Alex Vandiver)
+    * Slightly unify nigh-identical codepaths between Pg and Oracle
+      (Alex Vandiver)
 
 0.67 2011-02-28
 - Features:

Modified: trunk/libjifty-dbi-perl/META.yml
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/META.yml?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/META.yml (original)
+++ trunk/libjifty-dbi-perl/META.yml Tue Apr 19 21:39:40 2011
@@ -47,4 +47,4 @@
   version: 0
 resources:
   license: http://dev.perl.org/licenses/
-version: 0.67
+version: 0.68

Modified: trunk/libjifty-dbi-perl/SIGNATURE
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/SIGNATURE?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/SIGNATURE (original)
+++ trunk/libjifty-dbi-perl/SIGNATURE Tue Apr 19 21:39:40 2011
@@ -15,9 +15,9 @@
 Hash: SHA1
 
 SHA1 418a58763132c9a476627cbdce5ff01395ce84d4 .gitignore
-SHA1 45d9eaf8a622fdfc0bb9680396e2843eca5ec346 Changes
+SHA1 59a52f546dcd1cd87813bc59babf7b1cd32ac2d3 Changes
 SHA1 c2fb135f967d7093a6191d1b7e5e596e30040246 MANIFEST
-SHA1 f21d5f2dc45943fbc09ac079b2b18d942a6f3f81 META.yml
+SHA1 da76cdd7f1c89f107d3d5c9608aff9f886e503f8 META.yml
 SHA1 48bd6ca8a37ec79b7cae91028d7e9489ad33a03b Makefile.PL
 SHA1 e29d7b270f78a5a406921571b08290c46f2a42f6 README
 SHA1 82d6ac3f6def48558d09f8b6e3b53ed4194d8c81 ROADMAP
@@ -41,8 +41,8 @@
 SHA1 026cc0551a0ad399d195e395b46bdf842e115192 inc/Module/Install/Metadata.pm
 SHA1 5457015ea5a50e93465bf2dafa29feebd547f85b inc/Module/Install/Win32.pm
 SHA1 051e7fa8063908befa3440508d0584a2497b97db inc/Module/Install/WriteAll.pm
-SHA1 84ab56168fb14f1530c035b549c2af0750f0fd60 lib/Jifty/DBI.pm
-SHA1 e0375edf9f501e6b9c723fced70431108188419b lib/Jifty/DBI/Collection.pm
+SHA1 3a442252053b99436c8cdd084ced4801e4e22381 lib/Jifty/DBI.pm
+SHA1 fcab228fade86231a4a6024bd2c06813bbe4e555 lib/Jifty/DBI/Collection.pm
 SHA1 503ca4cf6693580dedf8adee58267532f8467908 lib/Jifty/DBI/Collection/Union.pm
 SHA1 bcba77fd2bacf0475aea1de97f57365c8de92ca6 lib/Jifty/DBI/Collection/Unique.pm
 SHA1 3ff96d74a769439111fba7b42b0c100d180ba6cd lib/Jifty/DBI/Column.pm
@@ -62,8 +62,8 @@
 SHA1 b043cbb2d750aa1b93e25718ec563d62b3cf13b8 lib/Jifty/DBI/Handle.pm
 SHA1 719a11c911aac5306baa4b44f683aa76261100c7 lib/Jifty/DBI/Handle/Informix.pm
 SHA1 338116a45f8eb6bfca5e76e8d3be78fb61fffe81 lib/Jifty/DBI/Handle/ODBC.pm
-SHA1 960fd0b63f3de11924c5d47a3c0c6d1db105ed5b lib/Jifty/DBI/Handle/Oracle.pm
-SHA1 d1757e2c992ead86f70f0dfc9c659387dc9600cf lib/Jifty/DBI/Handle/Pg.pm
+SHA1 8281a163b21bb4a5cb0f2b24ce4a55dab716c408 lib/Jifty/DBI/Handle/Oracle.pm
+SHA1 754666e0c41143aec23a34ea3326bf4fd1b8a24e lib/Jifty/DBI/Handle/Pg.pm
 SHA1 2f4c08340712bd21679282ebd669ce7b99d6d646 lib/Jifty/DBI/Handle/SQLite.pm
 SHA1 bba2314c20fcc3ef71cc69090f1cd6bd515cd9b4 lib/Jifty/DBI/Handle/Sybase.pm
 SHA1 cf80896a175702a157770f64ae469430678c3357 lib/Jifty/DBI/Handle/mysql.pm
@@ -120,9 +120,9 @@
 SHA1 97e60dd523a74a886c170eeb05b813aa551f5efe t/testmodels.pl
 SHA1 653c2f961d8b4f195e5391cd261f37815068e8d5 t/utils.pl
 -----BEGIN PGP SIGNATURE-----
-Version: GnuPG v1.4.10 (GNU/Linux)
+Version: GnuPG v1.4.10 (Darwin)
 
-iD8DBQFNa8XhHdv9ZfNcOAcRAmUeAJ4zMQK4gRNn+i7pu3EzlZN2jhXkpQCfcgER
-Z8gawoecAhAUhGjBuFm0yAM=
-=1fkV
+iEYEARECAAYFAk2nHj0ACgkQsxfQtHhyRPrWkQCfc7CyLd/KO0JgsR6RKGRhlkkh
+gyYAnioe5ZVx9iTThRBm2gmhnDZEDgfy
+=JFMa
 -----END PGP SIGNATURE-----

Modified: trunk/libjifty-dbi-perl/debian/changelog
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/debian/changelog?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/debian/changelog (original)
+++ trunk/libjifty-dbi-perl/debian/changelog Tue Apr 19 21:39:40 2011
@@ -1,3 +1,10 @@
+libjifty-dbi-perl (0.68-1) UNRELEASED; urgency=low
+
+  * New upstream release:
+    - fix SQL injection weaknesses (closes: #622919)
+
+ -- gregor herrmann <gregoa at debian.org>  Tue, 19 Apr 2011 23:36:57 +0200
+
 libjifty-dbi-perl (0.67-1) unstable; urgency=low
 
   * New upstream release

Modified: trunk/libjifty-dbi-perl/lib/Jifty/DBI.pm
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/lib/Jifty/DBI.pm?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/lib/Jifty/DBI.pm (original)
+++ trunk/libjifty-dbi-perl/lib/Jifty/DBI.pm Tue Apr 19 21:39:40 2011
@@ -2,7 +2,7 @@
 use warnings;
 use strict;
 
-$Jifty::DBI::VERSION = '0.67';
+$Jifty::DBI::VERSION = '0.68';
 
 =head1 NAME
 

Modified: trunk/libjifty-dbi-perl/lib/Jifty/DBI/Collection.pm
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/lib/Jifty/DBI/Collection.pm?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/lib/Jifty/DBI/Collection.pm (original)
+++ trunk/libjifty-dbi-perl/lib/Jifty/DBI/Collection.pm Tue Apr 19 21:39:40 2011
@@ -1253,15 +1253,6 @@
 
     # }}}
 
-    # 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;
-
     # $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.
@@ -1277,6 +1268,43 @@
         column    => $column_obj,
         value_ref => \$args{'value'},
     ) 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} );
@@ -1308,27 +1336,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'} );
         }
     }
 
@@ -1667,6 +1696,10 @@
         } 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'};
@@ -1751,6 +1784,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'} . ".";

Modified: trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Oracle.pm
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Oracle.pm?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Oracle.pm (original)
+++ trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Oracle.pm Tue Apr 19 21:39:40 2011
@@ -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;
     }

Modified: trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Pg.pm
URL: http://svn.debian.org/wsvn/pkg-perl/trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Pg.pm?rev=72824&op=diff
==============================================================================
--- trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Pg.pm (original)
+++ trunk/libjifty-dbi-perl/lib/Jifty/DBI/Handle/Pg.pm Tue Apr 19 21:39:40 2011
@@ -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;




More information about the Pkg-perl-cvs-commits mailing list