Cphalcon: ORM - Using limit with a negative number throws a MYSQL error

[jesusfreak3]

Expected

Calling ->limit($value) on a Query should parse all input to be a non negative number.

Actual Behavior

A user attempted a MYSQL injection where a limit value was passed to the API, the value passed was

Encoded:
limit=%3CMETA%20HTTP-EQUIV%3D%22refresh%22%20CONT ENT%3D
%220%3Burl%3Djavascript%3Aqss%3D7%22%3E

Decoded:

limit=<META HTTP-EQUIV="refresh" CONT ENT= "0;url=javascript:qss=7">

$query = SomeModel::query();   
//removed in between code
$query->limit($limit); 

and the ORM treated LIMIT input as an intval so it stripped all characters except numbers and negatives which then returned "-07"

MYSQL limit does not accept negative numbers, so throws and error, and Phalcon outputs the full query to the user which then exposes more details to the user attempting to sql inject.

Response:
Syntax error, unexpected token -, near to '07', when parsing: SELECT .....

#0 [internal function]: Phalcon\Mvc\Model\Query->parse()
#1 [internal function]: Phalcon\Mvc\Model\Query->execute(Array, NULL)
#2 [internal function]: Phalcon\Mvc\Model::find(Array)
#3 ...... : Phalcon\Mvc\Model\Criteria->execute()
#4 [internal function]: {closure}()
#5 ..../index.php(19): Phalcon\Mvc\Micro->handle()
#6 {main}

Details

• Phalcon version:
  Version => 3.0.1
  Build Date => Aug 24 2016 11:18:13

• PHP Version: (php -v)
  PHP 5.6.15

• Operating System:
  Windows, Ubuntu, Centos

• Installation type:
  DLL in windows, apt-get in Ubuntu etc..

• Zephir version (if any):
  Version 0.9.4a-dev-7e304ba18c

• Server:
  Nginx and Apache

[sergeyklay]

Thank for reporting. I'll fix it asap

[jesusfreak3]

NP, and thanks for the quick reply!

[Izopi4a]

use Phalcon\Db\Adapter\Pdo\Mysql as DbAdapter;
use Phalcon\DI\FactoryDefault;
use Phalcon\Mvc\Application;

$di = new FactoryDefault();

$di->set('db', function () {
    return new DbAdapter([
        'host' => "127.0.0.1",
        'username' => "root",
        'password' => "123456",
        'dbname' => "koko",
        "options" => [
            \PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
            \PDO::ATTR_EMULATE_PREPARES => false,
            \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC,
            \PDO::ATTR_STRINGIFY_FETCHES => false
        ]
    ]);
});

class koko extends \Phalcon\Mvc\Model {

    public function initialize()
    {
        $this->setSource("Persons");
    }
}

$application = new Application($di);


$query = koko::query();
//removed in between code
$query->limit(-7);
//$query->limit('<META HTTP-EQUIV="refresh" CONT ENT= "0;url=javascript:qss=7">');
//$query->limit("%3CMETA%20HTTP-EQUIV%3D%22refresh%22%20CONT ENT%3D%220%3Burl%3Djavascript%3Aqss%3D7%22%3E");

$a = $query->execute();

echo "<pre>";
var_dump($a->toArray()); //prints all my db results
echo "</pre>";
exit;

php 7 0 13
mysql 5 7 16

maybe we can tune a little bit here https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/model/criteria.zep#L512

and here https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/model/query/builder.zep#L941

to change var to int and offset only if limit is present stuff like that

what do you think @sergeyklay

[sergeyklay]

Fixed in the 3.0.x branch.