Merge branch 'security'

docs/guide/runtime-logging.md

@@ -216,6 +216,17 @@ You may configure `logVars` to be an empty array to totally disable the inclusio
 Or if you want to implement your own way of providing context information, you may override the
 [[yii\log\Target::getContextMessage()]] method.
 
+In case some of your request fields contain sensitive information you would not like to log (e.g. passwords, access tokens),
+you may additionally configure `maskVars` property. By default, the following request parameters will be masked with `***`:
+`$_SERVER[HTTP_AUTHORIZATION]`, `$_SERVER[PHP_AUTH_USER]`, `$_SERVER[PHP_AUTH_PW]`, but you can set your own:
+
+```php
+[
+    'class' => 'yii\log\FileTarget',
+    'logVars' => ['_SERVER'],
+    'maskVars' => ['_SERVER.HTTP_X_PASSWORD']
+]
+```
 
 ### Message Trace Level <span id="trace-level"></span>
 

framework/CHANGELOG.md

@@ -113,6 +113,9 @@ Yii Framework 2 Change Log
 - Bug #16959: Fixed typo in if condition inside `yii\web\DbSession::typecastFields()` that caused problems with session overwriting (silverfire)
 - Bug #15876: `yii\db\ActiveQuery::viaTable()` now throws `InvalidConfigException`, if query is not prepared correctly (silverfire)
 - Bug #15931: `yii\db\ActiveRecord::findOne()` now accepts quoted table and column names using curly and square braces respectively (silverfire)
+- Bug: (CVE-2018-14578): Fixed CSRF token check bypassing in `\yii\web\Request::getMethod()` (silverfire)
+- Bug: (CVE-2018-19454): Fixed excess logging of sensitive information in `\yii\log\Target` (silverfire)
+
 
 2.0.15.1 March 21, 2018
 -----------------------

framework/log/Target.php

@@ -73,7 +73,34 @@ abstract class Target extends Component
      *
      * @see \yii\helpers\ArrayHelper::filter()
      */
-    public $logVars = ['_GET', '_POST', '_FILES', '_COOKIE', '_SESSION', '_SERVER'];
+    public $logVars = [
+        '_GET',
+        '_POST',
+        '_FILES',
+        '_COOKIE',
+        '_SESSION',
+        '_SERVER',
+    ];
+
+    /**
+     * @var array list of the PHP predefined variables that should NOT be logged "as is" and should always be replaced
+     * with a mask `***` before logging, when exist.
+     *
+     * Defaults to `[ '_SERVER.HTTP_AUTHORIZATION', '_SERVER.PHP_AUTH_USER', '_SERVER.PHP_AUTH_PW']`
+     *
+     * Each element could be specified as one of the following:
+     *
+     * - `var` - `var` will be logged as `***`
+     * - `var.key` - only `var[key]` will be logged as `***`
+     *
+     * @since 2.0.16
+     */
+    public $maskVars = [
+        '_SERVER.HTTP_AUTHORIZATION',
+        '_SERVER.PHP_AUTH_USER',
+        '_SERVER.PHP_AUTH_PW',
+    ];
+
     /**
      * @var callable a PHP callable that returns a string to be prefixed to every exported message.
      *
@@ -145,6 +172,11 @@ public function collect($messages, $final)
     protected function getContextMessage()
     {
         $context = ArrayHelper::filter($GLOBALS, $this->logVars);
+        foreach ($this->maskVars as $var) {
+            if (ArrayHelper::getValue($context, $var) !== null) {
+                ArrayHelper::setValue($context, $var, '***');
+            }
+        }
         $result = [];
         foreach ($context as $key => $value) {
             $result[] = "\${$key} = " . VarDumper::dumpAsString($value);

framework/web/IdentityInterface.php

@@ -82,7 +82,12 @@ public function getId();
      *
      * The space of such keys should be big enough to defeat potential identity attacks.
      *
-     * This is required if [[User::enableAutoLogin]] is enabled.
+     * This is required if [[User::enableAutoLogin]] is enabled. The returned key will be stored on the
+     * client side as a cookie and will be used to authenticate user even if PHP session has been expired.
+     *
+     * Make sure to invalidate earlier issued authKeys when you implement force user logout, password change and
+     * other scenarios, that require forceful access revocation for old sessions.
+     *
      * @return string a key that is used to check the validity of a given identity ID.
      * @see validateAuthKey()
      */

framework/web/Request.php

@@ -371,7 +371,12 @@ public function getHeaders()
      */
     public function getMethod()
     {
-        if (isset($_POST[$this->methodParam])) {
+        if (
+            isset($_POST[$this->methodParam])
+            // Never allow to downgrade request from WRITE methods (POST, PATCH, DELETE, etc)
+            // to read methods (GET, HEAD, OPTIONS) for security reasons.
+            && !in_array(strtoupper($_POST[$this->methodParam]), ['GET', 'HEAD', 'OPTIONS'], true)
+        ) {
             return strtoupper($_POST[$this->methodParam]);
         }
 

tests/framework/log/TargetTest.php

@@ -92,6 +92,10 @@ public function testGetContextMessage()
                 'C', 'C.C_a',
                 'D',
             ],
+            'maskVars' => [
+                'C.C_b',
+                'D.D_a'
+            ]
         ]);
         $GLOBALS['A'] = [
             'A_a' => 1,
@@ -105,7 +109,7 @@ public function testGetContextMessage()
         ];
         $GLOBALS['C'] = [
             'C_a' => 1,
-            'C_b' => 1,
+            'C_b' => 'mySecret',
             'C_c' => 1,
         ];
         $GLOBALS['E'] = [
@@ -129,6 +133,8 @@ public function testGetContextMessage()
         $this->assertNotContains('E_a', $context);
         $this->assertNotContains('E_b', $context);
         $this->assertNotContains('E_c', $context);
+        $this->assertNotContains('mySecret', $context);
+        $this->assertContains('***', $context);
     }
 
     /**

tests/framework/web/RequestTest.php

@@ -724,4 +724,21 @@ public function testGetBodyParam()
         $this->assertSame(null, $request->getBodyParam('unexisting'));
         $this->assertSame('default', $request->getBodyParam('unexisting', 'default'));
     }
+
+    /**
+     * @testWith    ["POST", "GET", "POST"]
+     *              ["POST", "OPTIONS", "POST"]
+     *              ["POST", "HEAD", "POST"]
+     *              ["POST", "DELETE", "DELETE"]
+     *              ["POST", "CUSTOM", "CUSTOM"]
+     */
+    public function testRequestMethodCanNotBeDowngraded($requestMethod, $requestOverrideMethod, $expectedMethod)
+    {
+        $request = new Request();
+
+        $_SERVER['REQUEST_METHOD'] = $requestMethod;
+        $_POST[$request->methodParam] = $requestOverrideMethod;
+
+        $this->assertSame($expectedMethod, $request->getMethod());
+    }
 }