diff --git a/docs/guide/runtime-logging.md b/docs/guide/runtime-logging.md index bf226c8531e..815af1d455b 100644 --- a/docs/guide/runtime-logging.md +++ b/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> diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index f4329b7cbaa..1151acfb845 100644 --- a/framework/CHANGELOG.md +++ b/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 ----------------------- diff --git a/framework/log/Target.php b/framework/log/Target.php index c2d93449235..9675ddf70ce 100644 --- a/framework/log/Target.php +++ b/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); diff --git a/framework/web/IdentityInterface.php b/framework/web/IdentityInterface.php index cf5c0e28fff..fb5765d4764 100644 --- a/framework/web/IdentityInterface.php +++ b/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() */ diff --git a/framework/web/Request.php b/framework/web/Request.php index f6c1a0de183..b0ddd712034 100644 --- a/framework/web/Request.php +++ b/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]); } diff --git a/tests/framework/log/TargetTest.php b/tests/framework/log/TargetTest.php index 75040ae5cbd..b055da67291 100644 --- a/tests/framework/log/TargetTest.php +++ b/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); } /** diff --git a/tests/framework/web/RequestTest.php b/tests/framework/web/RequestTest.php index 6c50c4ae314..33877f55e61 100644 --- a/tests/framework/web/RequestTest.php +++ b/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()); + } }