Security advisory GHSA-7g7c-qhf3-x59p: propel/propel1 SQL injection possible with limit() on MySQL
Package: propel/propel1 (composer), vulnerable version: 1.7.1, fixed in: 1.7.2
CWE: CWE-89
Files changed by the fix:
generator/lib/behavior/sortable/SortableBehavior.php
runtime/lib/adapter/DBMySQL.php
runtime/lib/query/Criteria.php

Fix diff:
diff --git a/.travis.yml b/.travis.yml
index 07bfa5b50..8a7d36504 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -7,7 +7,6 @@ branches:
     - gh-pages
 
 php:
-  - 5.3
   - 5.4
   - 5.5
   - hhvm
@@ -26,8 +25,11 @@ before_script:
 
   - ./test/reset_tests.sh
 
-script: phpunit
+script: vendor/bin/phpunit
 
 matrix:
+  include:
+    - php: 5.3
+      dist: precise
   allow_failures:
     - php: hhvm
diff --git a/composer.json b/composer.json
index 0d9c39ac2..a5a42349a 100644
--- a/composer.json
+++ b/composer.json
@@ -18,7 +18,8 @@
     "phing/phing": "~2.4"
   },
   "require-dev": {
-    "pear-pear.php.net/PEAR_PackageFileManager2": "@stable"
+    "pear-pear.php.net/PEAR_PackageFileManager2": "@stable",
+    "phpunit/phpunit": "~4.0||~5.0"
   },
   "extra": {
     "branch-alias": {
@@ -28,7 +29,7 @@
   "repositories": [
     {
       "type": "pear",
-      "url": "http://pear.php.net"
+      "url": "https://pear.php.net"
     }
   ],
   "bin": ["generator/bin/propel-gen", "generator/bin/propel-gen.bat"]
diff --git a/generator/lib/behavior/sortable/SortableBehavior.php b/generator/lib/behavior/sortable/SortableBehavior.php
index 9c3117960..2bec8453d 100644
--- a/generator/lib/behavior/sortable/SortableBehavior.php
+++ b/generator/lib/behavior/sortable/SortableBehavior.php
@@ -96,6 +96,7 @@ public function generateScopePhp()
 
             $methodSignature = array();
             $buildScope      = array();
+            $buildScopeVars  = array();
             $paramsDoc       = array();
 
             foreach ($this->getScopes() as $idx => $scope) {
diff --git a/runtime/lib/adapter/DBMySQL.php b/runtime/lib/adapter/DBMySQL.php
index f629c6818..34ac19702 100644
--- a/runtime/lib/adapter/DBMySQL.php
+++ b/runtime/lib/adapter/DBMySQL.php
@@ -153,6 +153,9 @@ public function useQuoteIdentifier()
      */
     public function applyLimit(&$sql, $offset, $limit)
     {
+        $offset = (int) $offset;
+        $limit = (int) $limit;
+
         if ($limit > 0) {
             $sql .= " LIMIT " . ($offset > 0 ? $offset . ", " : "") . $limit;
         } elseif ($offset > 0) {
diff --git a/runtime/lib/query/Criteria.php b/runtime/lib/query/Criteria.php
index 005f83bcc..3c45ddf55 100644
--- a/runtime/lib/query/Criteria.php
+++ b/runtime/lib/query/Criteria.php
@@ -1239,8 +1239,7 @@ public function isSingleRecord()
      */
     public function setLimit($limit)
     {
-        // TODO: do we enforce int here? 32bit issue if we do
-        $this->limit = $limit;
+        $this->limit = (int) $limit;
 
         return $this;
     }
@@ -1258,8 +1257,7 @@ public function getLimit()
     /**
      * Set offset.
      *
-     * @param int $offset An int with the value for offset.  (Note this values is
-     *                    cast to a 32bit integer and may result in truncation)
+     * @param int $offset An int with the value for offset.
      *
      * @return Criteria Modified Criteria object (for fluent API)
      */
diff --git a/test/fixtures/reverse/mysql/build/sql/schema.sql b/test/fixtures/reverse/mysql/build/sql/schema.sql
index 4a2c2caaa..1b0d260d2 100644
--- a/test/fixtures/reverse/mysql/build/sql/schema.sql
+++ b/test/fixtures/reverse/mysql/build/sql/schema.sql
@@ -1,3 +1,5 @@
+SET NAMES utf8;
+
 DROP TABLE IF EXISTS book;
 DROP VIEW IF EXISTS view_book_titles;
 
diff --git a/test/testsuite/runtime/adapter/DBMySQLTest.php b/test/testsuite/runtime/adapter/DBMySQLTest.php
index 55d5b926d..72300a747 100644
--- a/test/testsuite/runtime/adapter/DBMySQLTest.php
+++ b/test/testsuite/runtime/adapter/DBMySQLTest.php
@@ -104,6 +104,194 @@ public function testQuotingIdentifiers()
         $db = new DBMySQL();
         $this->assertEquals('`Book ISBN`', $db->quoteIdentifier('Book ISBN'));
     }
+
+    /**
+     * @dataProvider dataApplyLimit
+     */
+    public function testApplyLimit($offset, $limit, $expectedSql)
+    {
+        $sql = '';
+
+        $db = new DBMySQL();
+        $db->applyLimit($sql, $offset, $limit);
+
+        $this->assertEquals($expectedSql, $sql, 'Generated SQL does not match expected SQL');
+    }
+
+    public function dataApplyLimit()
+    {
+        return array(
+
+            /*
+                Offset & limit = 0
+             */
+
+            'Zero offset & limit' => array(
+                'offset'      => 0,
+                'limit'       => 0,
+                'expectedSql' => ''
+            ),
+
+            /*
+                Offset = 0
+             */
+
+            '32-bit limit' => array(
+                'offset'      => 0,
+                'limit'       => 4294967295,
+                'expectedSql' => ' LIMIT 4294967295'
+            ),
+            '32-bit limit as a string' => array(
+                'offset'      => 0,
+                'limit'       => '4294967295',
+                'expectedSql' => ' LIMIT 4294967295'
+            ),
+
+            '64-bit limit' => array(
+                'offset'      => 0,
+                'limit'       => 9223372036854775807,
+                'expectedSql' => ' LIMIT 9223372036854775807'
+            ),
+            '64-bit limit as a string' => array(
+                'offset'      => 0,
+                'limit'       => '9223372036854775807',
+                'expectedSql' => ' LIMIT 9223372036854775807'
+            ),
+
+            'Float limit' => array(
+                'offset'      => 0,
+                'limit'       => 123.9,
+                'expectedSql' => ' LIMIT 123'
+            ),
+            'Float limit as a string' => array(
+                'offset'      => 0,
+                'limit'       => '123.9',
+                'expectedSql' => ' LIMIT 123'
+            ),
+
+            'Negative limit' => array(
+                'offset'      => 0,
+                'limit'       => -1,
+                'expectedSql' => ''
+            ),
+            'Non-numeric string limit' => array(
+                'offset'      => 0,
+                'limit'       => 'foo',
+                'expectedSql' => ''
+            ),
+            'SQL injected limit' => array(
+                'offset'      => 0,
+                'limit'       => '3;DROP TABLE abc',
+                'expectedSql' => ' LIMIT 3'
+            ),
+
+            /*
+                Limit = 0
+             */
+
+            '32-bit offset' => array(
+                'offset'      => 4294967295,
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 4294967295, 18446744073709551615'
+            ),
+            '32-bit offset as a string' => array(
+                'offset'      => '4294967295',
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 4294967295, 18446744073709551615'
+            ),
+
+            '64-bit offset' => array(
+                'offset'      => 9223372036854775807,
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 9223372036854775807, 18446744073709551615'
+            ),
+            '64-bit offset as a string' => array(
+                'offset'      => '9223372036854775807',
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 9223372036854775807, 18446744073709551615'
+            ),
+
+            'Float offset' => array(
+                'offset'      => 123.9,
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 123, 18446744073709551615'
+            ),
+            'Float offset as a string' => array(
+                'offset'      => '123.9',
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 123, 18446744073709551615'
+            ),
+
+            'Negative offset' => array(
+                'offset'      => -1,
+                'limit'       => 0,
+                'expectedSql' => ''
+            ),
+            'Non-numeric string offset' => array(
+                'offset'      => 'foo',
+                'limit'       => 0,
+                'expectedSql' => ''
+            ),
+            'SQL injected offset' => array(
+                'offset'      => '3;DROP TABLE abc',
+                'limit'       => 0,
+                'expectedSql' => ' LIMIT 3, 18446744073709551615'
+            ),
+
+            /*
+                Offset & limit != 0
+             */
+
+            array(
+                'offset'      => 4294967295,
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 4294967295, 999'
+            ),
+            array(
+                'offset'      => '4294967295',
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 4294967295, 999'
+            ),
+
+            array(
+                'offset'      => 9223372036854775807,
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 9223372036854775807, 999'
+            ),
+            array(
+                'offset'      => '9223372036854775807',
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 9223372036854775807, 999'
+            ),
+
+            array(
+                'offset'      => 123.9,
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 123, 999'
+            ),
+            array(
+                'offset'      => '123.9',
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 123, 999'
+            ),
+
+            array(
+                'offset'      => -1,
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 999'
+            ),
+            array(
+                'offset'      => 'foo',
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 999'
+            ),
+            array(
+                'offset'      => '3;DROP TABLE abc',
+                'limit'       => 999,
+                'expectedSql' => ' LIMIT 3, 999'
+            ),
+        );
+    }
 }
 
 // See: http://stackoverflow.com/questions/3138946/mocking-the-pdo-object-using-phpunit
diff --git a/test/testsuite/runtime/query/CriteriaTest.php b/test/testsuite/runtime/query/CriteriaTest.php
index 972cbb7a1..04a71e6a2 100644
--- a/test/testsuite/runtime/query/CriteriaTest.php
+++ b/test/testsuite/runtime/query/CriteriaTest.php
@@ -1154,16 +1154,160 @@ public function testClear()
         $this->assertFalse($c->getUseTransaction(), 'useTransaction is false by default');
     }
 
-    public function testLimit()
+    public function testDefaultLimit()
     {
         $c = new Criteria();
         $this->assertEquals(0, $c->getLimit(), 'Limit is 0 by default');
+    }
 
-        $c2 = $c->setLimit(1);
-        $this->assertEquals(1, $c->getLimit(), 'Limit is set by setLimit');
+    /**
+     * @dataProvider dataLimit
+     */
+    public function testLimit($limit, $expected)
+    {
+        $c = new Criteria();
+        $c2 = $c->setLimit($limit);
+
+        $this->assertSame($expected, $c->getLimit(), 'Correct limit is set by setLimit()');
         $this->assertSame($c, $c2, 'setLimit() returns the current Criteria');
     }
 
+    public function dataLimit()
+    {
+        return array(
+            'Negative value' => array(
+                'limit'    => -1,
+                'expected' => -1
+            ),
+            'Zero' => array(
+                'limit'    => 0,
+                'expected' => 0
+            ),
+
+            'Small integer' => array(
+                'limit'    => 38427,
+                'expected' => 38427
+            ),
+            'Small integer as a string' => array(
+                'limit'    => '38427',
+                'expected' => 38427
+            ),
+
+            'Largest 32-bit integer' => array(
+                'limit'    => 2147483647,
+                'expected' => 2147483647
+            ),
+            'Largest 32-bit integer as a string' => array(
+                'limit'    => '2147483647',
+                'expected' => 2147483647
+            ),
+
+            'Largest 64-bit integer' => array(
+                'limit'    => 9223372036854775807,
+                'expected' => 9223372036854775807
+            ),
+            'Largest 64-bit integer as a string' => array(
+                'limit'    => '9223372036854775807',
+                'expected' => 9223372036854775807
+            ),
+
+            'Decimal value' => array(
+                'limit'    => 123.9,
+                'expected' => 123
+            ),
+            'Decimal value as a string' => array(
+                'limit'    => '123.9',
+                'expected' => 123
+            ),
+
+            'Non-numeric string' => array(
+                'limit'    => 'foo',
+                'expected' => 0
+            ),
+            'Injected SQL' => array(
+                'limit'    => '3;DROP TABLE abc',
+                'expected' => 3
+            ),
+        );
+    }
+
+    public function testDefaultOffset()
+    {
+        $c = new Criteria();
+        $this->assertEquals(0, $c->getOffset(), 'Offset is 0 by default');
+    }
+
+    /**
+     * @dataProvider dataOffset
+     */
+    public function testOffset($offset, $expected)
+    {
+        $c = new Criteria();
+        $c2 = $c->setOffset($offset);
+
+        $this->assertSame($expected, $c->getOffset(), 'Correct offset is set by setOffset()');
+        $this->assertSame($c, $c2, 'setOffset() returns the current Criteria');
+    }
+
+    public function dataOffset()
+    {
+        return array(
+            'Negative value' => array(
+                'offset'   => -1,
+                'expected' => -1
+            ),
+            'Zero' => array(
+                'offset'   => 0,
+                'expected' => 0
+            ),
+
+            'Small integer' => array(
+                'offset'   => 38427,
+                'expected' => 38427
+            ),
+            'Small integer as a string' => array(
+                'offset'   => '38427',
+                'expected' => 38427
+            ),
+
+            'Largest 32-bit integer' => array(
+                'offset'   => 2147483647,
+                'expected' => 2147483647
+            ),
+            'Largest 32-bit integer as a string' => array(
+                'offset'   => '2147483647',
+                'expected' => 2147483647
+            ),
+
+            'Largest 64-bit integer' => array(
+                'offset'   => 9223372036854775807,
+                'expected' => 9223372036854775807
+            ),
+            'Largest 64-bit integer as a string' => array(
+                'offset'   => '9223372036854775807',
+                'expected' => 9223372036854775807
+            ),
+
+            'Decimal value' => array(
+                'offset'   => 123.9,
+                'expected' => 123
+            ),
+            'Decimal value as a string' => array(
+                'offset'   => '123.9',
+                'expected' => 123
+            ),
+
+            'Non-numeric string' => array(
+                'offset'   => 'foo',
+                'expected' => 0
+            ),
+            'Injected SQL' => array(
+                'offset'   => '3;DROP TABLE abc',
+                'expected' => 3
+            ),
+        );
+    }
+
     public function testDistinct()
     {
         $c = new Criteria();