diff --git a/app/Config/core.php b/app/Config/core.php index e8dc9afda..da5555a34 100644 --- a/app/Config/core.php +++ b/app/Config/core.php @@ -198,6 +198,7 @@ * * - `Session.cookie` - The name of the cookie to use. Defaults to 'CAKEPHP' * - `Session.timeout` - The number of minutes you want sessions to live for. This timeout is handled by CakePHP + * - `Session.useForwardsCompatibleTimeout` - Whether or not to make timeout 3.x compatible. * - `Session.cookieTimeout` - The number of minutes you want session cookies to live for. * - `Session.checkAgent` - Do you want the user agent to be checked when starting sessions? You might want to set the * value to false, when dealing with older versions of IE, Chrome Frame or certain web-browsing devices and AJAX diff --git a/lib/Cake/Controller/Component/AuthComponent.php b/lib/Cake/Controller/Component/AuthComponent.php index e21f6e085..a652e950a 100644 --- a/lib/Cake/Controller/Component/AuthComponent.php +++ b/lib/Cake/Controller/Component/AuthComponent.php @@ -298,6 +298,7 @@ class AuthComponent extends Component { } if ($this->_isAllowed($controller)) { + $this->_getUser(); return true; } diff --git a/lib/Cake/Controller/Controller.php b/lib/Cake/Controller/Controller.php index 41c848a7d..e9d5c6657 100644 --- a/lib/Cake/Controller/Controller.php +++ b/lib/Cake/Controller/Controller.php @@ -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) { diff --git a/lib/Cake/Core/CakeObject.php b/lib/Cake/Core/CakeObject.php index 5c6b1e3d8..0a0f3ac16 100644 --- a/lib/Cake/Core/CakeObject.php +++ b/lib/Cake/Core/CakeObject.php @@ -148,7 +148,7 @@ class CakeObject { * Convenience method to write a message to CakeLog. See CakeLog::write() * for more information on writing to logs. * - * @param string $msg Log message + * @param mixed $msg Log message * @param int $type Error type constant. Defined in app/Config/core.php. * @param null|string|array $scope The scope(s) a log message is being created in. * See CakeLog::config() for more information on logging scopes. diff --git a/lib/Cake/Model/Datasource/CakeSession.php b/lib/Cake/Model/Datasource/CakeSession.php index a692ed74c..8e9429b53 100644 --- a/lib/Cake/Model/Datasource/CakeSession.php +++ b/lib/Cake/Model/Datasource/CakeSession.php @@ -134,6 +134,13 @@ class CakeSession { */ protected static $_cookieName = null; +/** + * Whether or not to make `_validAgentAndTime` 3.x compatible. + * + * @var bool + */ + protected static $_useForwardsCompatibleTimeout = false; + /** * Whether this session is running under a CLI environment * @@ -360,6 +367,9 @@ class CakeSession { protected static function _validAgentAndTime() { $userAgent = static::read('Config.userAgent'); $time = static::read('Config.time'); + if (static::$_useForwardsCompatibleTimeout) { + $time += (Configure::read('Session.timeout') * 60); + } $validAgent = ( Configure::read('Session.checkAgent') === false || isset($userAgent) && static::$_userAgent === $userAgent @@ -527,6 +537,10 @@ class CakeSession { if (isset($sessionConfig['timeout']) && !isset($sessionConfig['cookieTimeout'])) { $sessionConfig['cookieTimeout'] = $sessionConfig['timeout']; } + if (isset($sessionConfig['useForwardsCompatibleTimeout']) && $sessionConfig['useForwardsCompatibleTimeout']) { + static::$_useForwardsCompatibleTimeout = true; + } + if (!isset($sessionConfig['ini']['session.cookie_lifetime'])) { $sessionConfig['ini']['session.cookie_lifetime'] = $sessionConfig['cookieTimeout'] * 60; } @@ -579,7 +593,10 @@ class CakeSession { ); } Configure::write('Session', $sessionConfig); - static::$sessionTime = static::$time + ($sessionConfig['timeout'] * 60); + static::$sessionTime = static::$time; + if (!static::$_useForwardsCompatibleTimeout) { + static::$sessionTime += ($sessionConfig['timeout'] * 60); + } } /** diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 3813e3883..cb0b6ef83 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -1695,6 +1695,8 @@ class Model extends CakeObject implements CakeEventListener { * If an array, allows control of 'validate', 'callbacks' and 'counterCache' options. * See Model::save() for details of each options. * @return bool|array See Model::save() False on failure or an array of model data on success. + * @deprecated 3.0.0 To ease migration to the new major, do not use this method anymore. + * Stateful model usage will be removed. Use the existing save() methods instead. * @see Model::save() * @link https://book.cakephp.org/2.0/en/models/saving-your-data.html#model-savefield-string-fieldname-string-fieldvalue-validate-false */ @@ -3083,7 +3085,11 @@ class Model extends CakeObject implements CakeEventListener { $query['order'] = $this->order; } - $query['order'] = (array)$query['order']; + if (is_object($query['order'])) { + $query['order'] = array($query['order']); + } else { + $query['order'] = (array)$query['order']; + } if ($query['callbacks'] === true || $query['callbacks'] === 'before') { $event = new CakeEvent('Model.beforeFind', $this, array($query)); diff --git a/lib/Cake/Network/CakeRequest.php b/lib/Cake/Network/CakeRequest.php index 20589cb34..228e46e2e 100644 --- a/lib/Cake/Network/CakeRequest.php +++ b/lib/Cake/Network/CakeRequest.php @@ -439,7 +439,7 @@ class CakeRequest implements ArrayAccess { if (!empty($ref) && !empty($base)) { if ($local && strpos($ref, $base) === 0) { $ref = substr($ref, strlen($base)); - if (empty($ref)) { + if (!strlen($ref) || strpos($ref, '//') === 0) { $ref = '/'; } if ($ref[0] !== '/') { diff --git a/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php b/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php index 1e5d79f2a..09e99c92e 100644 --- a/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php @@ -1818,4 +1818,38 @@ class AuthComponentTest extends CakeTestCase { $this->assertEquals('/users/login', $this->Controller->testUrl); } + +/** + * testStatelessAuthAllowedActionsRetrieveUser method + * + * @return void + */ + public function testStatelessAuthAllowedActionsRetrieveUser() { + if (CakeSession::id()) { + session_destroy(); + CakeSession::$id = null; + } + $_SESSION = null; + + $_SERVER['PHP_AUTH_USER'] = 'mariano'; + $_SERVER['PHP_AUTH_PW'] = 'cake'; + + AuthComponent::$sessionKey = false; + $this->Controller->Auth->authenticate = array( + 'Basic' => array('userModel' => 'AuthUser') + ); + $this->Controller->request['action'] = 'add'; + $this->Controller->Auth->initialize($this->Controller); + $this->Controller->Auth->allow(); + $this->Controller->Auth->startup($this->Controller); + + $expectedUser = array( + 'id' => '1', + 'username' => 'mariano', + 'created' => '2007-03-17 01:16:23', + 'updated' => '2007-03-17 01:18:31', + ); + + $this->assertEquals($expectedUser, $this->Controller->Auth->user()); + } } diff --git a/lib/Cake/Test/Case/Controller/ControllerTest.php b/lib/Cake/Test/Case/Controller/ControllerTest.php index ef157ddd1..686520df6 100644 --- a/lib/Cake/Test/Case/Controller/ControllerTest.php +++ b/lib/Cake/Test/Case/Controller/ControllerTest.php @@ -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 * diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index b42239f08..851290547 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -7423,7 +7423,7 @@ class ModelReadTest extends BaseModelTest { } /** - * Test find(count) with Db::expression + * Test find(count) with DboSource::expression * * @return void */ @@ -7445,6 +7445,38 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals(1, $result); } +/** + * Test 'order' with DboSource::expression + */ + public function testOrderWithDbExpressions() { + $this->loadFixtures('User'); + + $User = new User(); + + $results = $User->find('all', array( + 'fields' => array('id'), + 'recursive' => -1, + 'order' => $this->db->expression('CASE id WHEN 4 THEN 0 ELSE id END'), + )); + + $expected = array( + array( + 'User' => array('id' => 4), + ), + array( + 'User' => array('id' => 1), + ), + array( + 'User' => array('id' => 2), + ), + array( + 'User' => array('id' => 3), + ), + ); + + $this->assertEquals($expected, $results); + } + /** * testFindMagic method * diff --git a/lib/Cake/Test/Case/Network/CakeRequestTest.php b/lib/Cake/Test/Case/Network/CakeRequestTest.php index 2ae40c980..abdb79be8 100644 --- a/lib/Cake/Test/Case/Network/CakeRequestTest.php +++ b/lib/Cake/Test/Case/Network/CakeRequestTest.php @@ -739,6 +739,9 @@ class CakeRequestTest extends CakeTestCase { $result = $request->referer(); $this->assertSame($result, 'https://cakephp.org'); + $result = $request->referer(true); + $this->assertSame('/', $result); + $_SERVER['HTTP_REFERER'] = ''; $result = $request->referer(); $this->assertSame($result, '/'); @@ -751,6 +754,18 @@ class CakeRequestTest extends CakeTestCase { $result = $request->referer(true); $this->assertSame($result, '/some/path'); + $_SERVER['HTTP_REFERER'] = Configure::read('App.fullBaseUrl') . '///cakephp.org/'; + $result = $request->referer(true); + $this->assertSame('/', $result); // Avoid returning scheme-relative URLs. + + $_SERVER['HTTP_REFERER'] = Configure::read('App.fullBaseUrl') . '/0'; + $result = $request->referer(true); + $this->assertSame('/0', $result); + + $_SERVER['HTTP_REFERER'] = Configure::read('App.fullBaseUrl') . '/'; + $result = $request->referer(true); + $this->assertSame('/', $result); + $_SERVER['HTTP_REFERER'] = Configure::read('App.fullBaseUrl') . '/some/path'; $result = $request->referer(false); $this->assertSame($result, Configure::read('App.fullBaseUrl') . '/some/path'); diff --git a/lib/Cake/TestSuite/Reporter/CakeHtmlReporter.php b/lib/Cake/TestSuite/Reporter/CakeHtmlReporter.php index 8c07e9b45..f73bb1927 100644 --- a/lib/Cake/TestSuite/Reporter/CakeHtmlReporter.php +++ b/lib/Cake/TestSuite/Reporter/CakeHtmlReporter.php @@ -38,6 +38,7 @@ class CakeHtmlReporter extends CakeBaseReporter { $this->paintDocumentStart(); $this->paintTestMenu(); echo "