Multiple so called 'type juggling' attacks. Most notably PhabricatorUser::validateCSRFToken() is 'bypassable' in certain cases.

/* MONGOOSE mongoose MONGOOSE mongoose MONGOOSE mongoose MONGOOSE mongoose */ The Phabricator code base is at various places vulnerable for so called 'type juggling' [1] attacks. Most notably PhabricatorUser::validateCSRFToken() is 'bypassable' in certain cases. ##Type Juggling Since PHP's loose type comparison operators compare only data values but not their associated types, deriving variable types from context. PHP's string conversion rules [2] specify strings (when evaluated in a numeric context) with leading decimal, hexadecimal, infinity, NAN or radix (a '.') data optionally followed by an exponent are evaluated as floats. What this means is that a string like 00e13242 is cast to 0 and as such, to PHP 0e94323 == 00e19384. Translated into code, this means that the following comparison: (hash($randomSecret) == $user_input) will return true if hash($randomSecret) returns a hash in the form 0+[eE]\d+ and the $user_input is given as "0". This applies to various hashing algorithms, including (but not limited to) MD5 and SHA1. If you're not convinced, try running the PHP-code at the bottom of this text for a PoC with SHA1. ##How this applies to Phabricator In order to understand how this applies to Phabricator, we need to first know that static method PhabricatorHash::digest() located at src/infrastructure/util/PhabricatorHash.php [3] on line 25 returns a sha1 hash via hash_hmac(); Then we need to know that in AphrontRequest::validateCSRF();[4] the static function PhabricatorUser::validateCSRFToken($token); is called with a user-supplied $token. Now, if we look at PhabricatorUser::validateCSRFToken() located at phabricator/src/applications/people/storage/PhabricatorUser.php [5], on line 409 we see that getRawCSRFToken (which also returns a sha1 hash) is called and it's return value is stored in $valid $valid = $this->getRawCSRFToken($ii); then on line 412 we see: if ($token == $valid) { or, if the $version is 'breach' we reach this code on line 419-420: $digest = PhabricatorHash::digest($valid, $salt); if (substr($digest, 0, self::CSRF_TOKEN_LENGTH) == $token) { Both these comparisons meets the criteria for type juggling. Since $valid is a randomly generated sha1 hash which eventually will be of the form 0+[eE]\d+ after sufficient regenerations and the same goes for $digest. In addition, $token is a user-supplied value. ##TL;DR So in short, some expected CSRF-tokens will equal a user-supplied CSRF-token containing "0". In essence this is a reduction in entropy of the CSRF-tokens. to stop PHP from automatically casting either value to another type during comparison, simply change: if ($token == $valid) { to if ($token === $valid) { And likewise: if (substr($digest, 0, self::CSRF_TOKEN_LENGTH) == $token) { to if (substr($digest, 0, self::CSRF_TOKEN_LENGTH) === $token) { In addition to the above vulnerability, there are other notable misuses of PHP comparison operators. phabricator/src/applications/conduit/method/ConduitConnectConduitAPIMethod.php:144 in protected function execute(ConduitAPIRequest $request) [6]: $valid = sha1($token.$user->getConduitCertificate()); if ($valid != $signature) { throw new ConduitException('ERR-INVALID-CERTIFICATE'); The != operator is not type strict and $signature is user-specified. $token is also user-specified. To fix this != should be changed to !==. phabricator/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php in verifyMessage() on line 15 and 16 [7]: $sig = $request->getStr('signature'); return hash_hmac('sha256', $timestamp.$token, $api_key) == $sig Where $sig is obviously a user-supplied value and can thus be 0, and again the return value of hash_hmac can be of the form 0+[eE]\d+. To fix this, change: hash_hmac('sha256', $timestamp.$token, $api_key) == $sig to: hash_hmac('sha256', $timestamp.$token, $api_key) === $sig ##Examples: - type juggling with sha1[8] <?php /* sha1 type juggling PoC values */ $v1 = sha1("AAJd1x3j"); $v2 = sha1("AAPkbYlH"); $v3 = sha1("AAZlIwOZ"); var_dump($v1, $v2, $v3); /* wrong way to compare these hashes */ var_dump( $v1 == $v2, $v2 == $v3, $v3 == $v1, $v1 == "0"); /* correct way */ var_dump( $v1 === $v2, $v2 === $v3, $v3 === $v1 ); ?> The above will output: string(40) "00e6811279456694288001763399976992804485" string(40) "0e51223820731210116366152413868569204545" string(40) "0e13965443605273185827757762777509208778" bool(true) bool(true) bool(true) bool(true) bool(false) bool(false) bool(false) ##References: 1. http://turbochaos.blogspot.nl/2013/08/exploiting-exotic-bugs-php-type-juggling.html 2. http://php.net/manual/en/language.types.string.php#language.types.string.conversion 3. https://github.com/phacility/phabricator/blob/master/src/infrastructure/util/PhabricatorHash.php#L25 4. https://github.com/phacility/phabricator/blob/master/src/aphront/AphrontRequest.php#L249 5. https://github.com/phacility/phabricator/blob/master/src/applications/people/storage/PhabricatorUser.php 6. https://github.com/phacility/phabricator/blob/master//src/applications/conduit/method/ConduitConnectConduitAPIMethod.php#L145 7. https://github.com/phacility/phabricator/blob/master/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php#L15 8. https://pen-testing.sans.org/blog/pen-testing/2014/12/18/php-weak-typing-woes-with-some-pontification-about-code-and-pen-testing [1]: http://turbochaos.blogspot.nl/2013/08/exploiting-exotic-bugs-php-type-juggling.html "1" [2]: http://php.net/manual/en/language.types.string.php#language.types.string.conversion "2" [3]: https://github.com/phacility/phabricator/blob/master/src/infrastructure/util/PhabricatorHash.php#L25 "3" [4]: https://github.com/phacility/phabricator/blob/master/src/aphront/AphrontRequest.php#L249 "4" [5]: https://github.com/phacility/phabricator/blob/master/src/applications/people/storage/PhabricatorUser.php "5" [6]: https://github.com/phacility/phabricator/blob/master//src/applications/conduit/method/ConduitConnectConduitAPIMethod.php#L145 "6" [7]: https://github.com/phacility/phabricator/blob/master/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php#L15 "7" [8]: https://pen-testing.sans.org/blog/pen-testing/2014/12/18/php-weak-typing-woes-with-some-pontification-about-code-and-pen-testing "8" /*END MONGOOSE mongoose MONGOOSE mongoose MONGOOSE mongoose MONGOOSE mongoose */