Merge pull request #11526 from cakephp/post-conditions

Make postConditions() less permissive.
This commit is contained in:
Mark Story 2017-12-15 14:36:38 -05:00 committed by GitHub
commit 3bf93b7f76
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 1 deletions

View file

@ -1018,7 +1018,12 @@ class Controller extends CakeObject implements CakeEventListener {
}
/**
* Converts POST'ed form data to a model conditions array, suitable for use in a Model::find() call.
* Converts POST'ed form data to a model conditions array.
*
* If combined with SecurityComponent these conditions could be suitable
* for use in a Model::find() call. Without SecurityComponent this method
* is vulnerable creating conditions containing SQL injection. While we
* attempt to raise exceptions.
*
* @param array $data POST'ed data organized by model and field
* @param string|array $op A string containing an SQL comparison operator, or an array matching operators
@ -1028,6 +1033,7 @@ class Controller extends CakeObject implements CakeEventListener {
* included in the returned conditions
* @return array|null An array of model conditions
* @deprecated 3.0.0 Will be removed in 3.0.
* @throws RuntimeException when unsafe operators are found.
*/
public function postConditions($data = array(), $op = null, $bool = 'AND', $exclusive = false) {
if (!is_array($data) || empty($data)) {
@ -1043,9 +1049,16 @@ class Controller extends CakeObject implements CakeEventListener {
$op = '';
}
$allowedChars = '#[^a-zA-Z0-9_ ]#';
$arrayOp = is_array($op);
foreach ($data as $model => $fields) {
if (preg_match($allowedChars, $model)) {
throw new RuntimeException("Unsafe operator found in {$model}");
}
foreach ($fields as $field => $value) {
if (preg_match($allowedChars, $field)) {
throw new RuntimeException("Unsafe operator found in {$model}.{$field}");
}
$key = $model . '.' . $field;
$fieldOp = $op;
if ($arrayOp) {

View file

@ -1177,6 +1177,48 @@ class ControllerTest extends CakeTestCase {
$this->assertSame($expected, $result);
}
/**
* data provider for dangerous post conditions.
*
* @return array
*/
public function dangerousPostConditionsProvider() {
return array(
array(
array('Model' => array('field !=' => 1))
),
array(
array('Model' => array('field AND 1=1 OR' => 'thing'))
),
array(
array('Model' => array('field >' => 1))
),
array(
array('Model' => array('field OR RAND()' => 1))
),
array(
array('Posts' => array('id IS NULL union all select posts.* from posts where id; --' => 1))
),
array(
array('Post.id IS NULL; --' => array('id' => 1))
),
);
}
/**
* test postConditions raising an exception on unsafe keys.
*
* @expectedException RuntimeException
* @dataProvider dangerousPostConditionsProvider
* @return void
*/
public function testPostConditionsDangerous($data) {
$request = new CakeRequest('controller_posts/index');
$Controller = new Controller($request);
$Controller->postConditions($data);
}
/**
* testControllerHttpCodes method
*