From 811cdcdfcb0442791b0c6dfd8814c621a44731cc Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Thu, 10 Oct 2019 15:21:41 -0800 Subject: [PATCH 1/6] Fix security vulnerbilities. Fix possible length extension attack, predicable generators, timing attacks on hash comparision and improved formatting. --- library/OAuth1.php | 371 ++++++++++++++++++------------- mod/lostpass.php | 3 +- src/Core/Authentication.php | 20 +- src/Model/Register.php | 3 +- src/Model/User.php | 92 +++++--- src/Module/Login.php | 32 ++- src/Network/FKOAuthDataStore.php | 8 +- src/Util/Strings.php | 258 ++++++++++----------- 8 files changed, 455 insertions(+), 332 deletions(-) diff --git a/library/OAuth1.php b/library/OAuth1.php index 27ee090b1..723f6592e 100644 --- a/library/OAuth1.php +++ b/library/OAuth1.php @@ -4,27 +4,30 @@ /* Generic exception class */ if (!class_exists('OAuthException', false)) { - class OAuthException extends Exception { - // pass - } + class OAuthException extends Exception + { } } -class OAuthConsumer { +class OAuthConsumer +{ public $key; public $secret; - function __construct($key, $secret, $callback_url=NULL) { + function __construct($key, $secret, $callback_url = NULL) + { $this->key = $key; $this->secret = $secret; $this->callback_url = $callback_url; } - function __toString() { + function __toString() + { return "OAuthConsumer[key=$this->key,secret=$this->secret]"; } } -class OAuthToken { +class OAuthToken +{ // access tokens and request tokens public $key; public $secret; @@ -37,7 +40,8 @@ class OAuthToken { * key = the token * secret = the token secret */ - function __construct($key, $secret) { + function __construct($key, $secret) + { $this->key = $key; $this->secret = $secret; } @@ -46,14 +50,16 @@ class OAuthToken { * generates the basic string serialization of a token that a server * would respond to request_token and access_token calls with */ - function to_string() { + function to_string() + { return "oauth_token=" . - OAuthUtil::urlencode_rfc3986($this->key) . - "&oauth_token_secret=" . - OAuthUtil::urlencode_rfc3986($this->secret); + OAuthUtil::urlencode_rfc3986($this->key) . + "&oauth_token_secret=" . + OAuthUtil::urlencode_rfc3986($this->secret); } - function __toString() { + function __toString() + { return $this->to_string(); } } @@ -62,7 +68,8 @@ class OAuthToken { * A class for implementing a Signature Method * See section 9 ("Signing Requests") in the spec */ -abstract class OAuthSignatureMethod { +abstract class OAuthSignatureMethod +{ /** * Needs to return the name of the Signature Method (ie HMAC-SHA1) * @return string @@ -89,25 +96,29 @@ abstract class OAuthSignatureMethod { * @param string $signature * @return bool */ - public function check_signature($request, $consumer, $token, $signature) { + public function check_signature($request, $consumer, $token, $signature) + { $built = $this->build_signature($request, $consumer, $token); return ($built == $signature); } } /** - * The HMAC-SHA1 signature method uses the HMAC-SHA1 signature algorithm as defined in [RFC2104] - * where the Signature Base String is the text and the key is the concatenated values (each first - * encoded per Parameter Encoding) of the Consumer Secret and Token Secret, separated by an '&' + * The HMAC-SHA1 signature method uses the HMAC-SHA1 signature algorithm as defined in [RFC2104] + * where the Signature Base String is the text and the key is the concatenated values (each first + * encoded per Parameter Encoding) of the Consumer Secret and Token Secret, separated by an '&' * character (ASCII code 38) even if empty. * - Chapter 9.2 ("HMAC-SHA1") */ -class OAuthSignatureMethod_HMAC_SHA1 extends OAuthSignatureMethod { - function get_name() { +class OAuthSignatureMethod_HMAC_SHA1 extends OAuthSignatureMethod +{ + function get_name() + { return "HMAC-SHA1"; } - public function build_signature($request, $consumer, $token) { + public function build_signature($request, $consumer, $token) + { $base_string = $request->get_signature_base_string(); $request->base_string = $base_string; @@ -126,25 +137,28 @@ class OAuthSignatureMethod_HMAC_SHA1 extends OAuthSignatureMethod { } /** - * The PLAINTEXT method does not provide any security protection and SHOULD only be used + * The PLAINTEXT method does not provide any security protection and SHOULD only be used * over a secure channel such as HTTPS. It does not use the Signature Base String. * - Chapter 9.4 ("PLAINTEXT") */ -class OAuthSignatureMethod_PLAINTEXT extends OAuthSignatureMethod { - public function get_name() { +class OAuthSignatureMethod_PLAINTEXT extends OAuthSignatureMethod +{ + public function get_name() + { return "PLAINTEXT"; } /** - * oauth_signature is set to the concatenated encoded values of the Consumer Secret and - * Token Secret, separated by a '&' character (ASCII code 38), even if either secret is + * oauth_signature is set to the concatenated encoded values of the Consumer Secret and + * Token Secret, separated by a '&' character (ASCII code 38), even if either secret is * empty. The result MUST be encoded again. * - Chapter 9.4.1 ("Generating Signatures") * * Please note that the second encoding MUST NOT happen in the SignatureMethod, as * OAuthRequest handles this! */ - public function build_signature($request, $consumer, $token) { + public function build_signature($request, $consumer, $token) + { $key_parts = array( $consumer->secret, ($token) ? $token->secret : "" @@ -159,15 +173,17 @@ class OAuthSignatureMethod_PLAINTEXT extends OAuthSignatureMethod { } /** - * The RSA-SHA1 signature method uses the RSASSA-PKCS1-v1_5 signature algorithm as defined in - * [RFC3447] section 8.2 (more simply known as PKCS#1), using SHA-1 as the hash function for - * EMSA-PKCS1-v1_5. It is assumed that the Consumer has provided its RSA public key in a - * verified way to the Service Provider, in a manner which is beyond the scope of this + * The RSA-SHA1 signature method uses the RSASSA-PKCS1-v1_5 signature algorithm as defined in + * [RFC3447] section 8.2 (more simply known as PKCS#1), using SHA-1 as the hash function for + * EMSA-PKCS1-v1_5. It is assumed that the Consumer has provided its RSA public key in a + * verified way to the Service Provider, in a manner which is beyond the scope of this * specification. * - Chapter 9.3 ("RSA-SHA1") */ -abstract class OAuthSignatureMethod_RSA_SHA1 extends OAuthSignatureMethod { - public function get_name() { +abstract class OAuthSignatureMethod_RSA_SHA1 extends OAuthSignatureMethod +{ + public function get_name() + { return "RSA-SHA1"; } @@ -185,7 +201,8 @@ abstract class OAuthSignatureMethod_RSA_SHA1 extends OAuthSignatureMethod { // Either way should return a string representation of the certificate protected abstract function fetch_private_cert(&$request); - public function build_signature($request, $consumer, $token) { + public function build_signature($request, $consumer, $token) + { $base_string = $request->get_signature_base_string(); $request->base_string = $base_string; @@ -204,7 +221,8 @@ abstract class OAuthSignatureMethod_RSA_SHA1 extends OAuthSignatureMethod { return base64_encode($signature); } - public function check_signature($request, $consumer, $token, $signature) { + public function check_signature($request, $consumer, $token, $signature) + { $decoded_sig = base64_decode($signature); $base_string = $request->get_signature_base_string(); @@ -225,7 +243,8 @@ abstract class OAuthSignatureMethod_RSA_SHA1 extends OAuthSignatureMethod { } } -class OAuthRequest { +class OAuthRequest +{ private $parameters; private $http_method; private $http_url; @@ -234,9 +253,10 @@ class OAuthRequest { public static $version = '1.0'; public static $POST_INPUT = 'php://input'; - function __construct($http_method, $http_url, $parameters=NULL) { + function __construct($http_method, $http_url, $parameters = NULL) + { @$parameters or $parameters = array(); - $parameters = array_merge( OAuthUtil::parse_parameters(parse_url($http_url, PHP_URL_QUERY)), $parameters); + $parameters = array_merge(OAuthUtil::parse_parameters(parse_url($http_url, PHP_URL_QUERY)), $parameters); $this->parameters = $parameters; $this->http_method = $http_method; $this->http_url = $http_url; @@ -246,15 +266,16 @@ class OAuthRequest { /** * attempt to build up a request from what was passed to the server */ - public static function from_request($http_method=NULL, $http_url=NULL, $parameters=NULL) { + public static function from_request($http_method = NULL, $http_url = NULL, $parameters = NULL) + { $scheme = (!isset($_SERVER['HTTPS']) || $_SERVER['HTTPS'] != "on") - ? 'http' - : 'https'; + ? 'http' + : 'https'; @$http_url or $http_url = $scheme . - '://' . $_SERVER['HTTP_HOST'] . - ':' . - $_SERVER['SERVER_PORT'] . - $_SERVER['REQUEST_URI']; + '://' . $_SERVER['HTTP_HOST'] . + ':' . + $_SERVER['SERVER_PORT'] . + $_SERVER['REQUEST_URI']; @$http_method or $http_method = $_SERVER['REQUEST_METHOD']; // We weren't handed any parameters, so let's find the ones relevant to @@ -270,10 +291,13 @@ class OAuthRequest { // It's a POST request of the proper content-type, so parse POST // parameters and add those overriding any duplicates from GET - if ($http_method == "POST" - && @strstr($request_headers["Content-Type"], - "application/x-www-form-urlencoded") - ) { + if ( + $http_method == "POST" + && @strstr( + $request_headers["Content-Type"], + "application/x-www-form-urlencoded" + ) + ) { $post_data = OAuthUtil::parse_parameters( file_get_contents(self::$POST_INPUT) ); @@ -288,25 +312,27 @@ class OAuthRequest { ); $parameters = array_merge($parameters, $header_parameters); } - } // fix for friendica redirect system - - $http_url = substr($http_url, 0, strpos($http_url,$parameters['pagename'])+strlen($parameters['pagename'])); - unset( $parameters['pagename'] ); - + + $http_url = substr($http_url, 0, strpos($http_url, $parameters['pagename']) + strlen($parameters['pagename'])); + unset($parameters['pagename']); + return new OAuthRequest($http_method, $http_url, $parameters); } /** * pretty much a helper function to set up the request */ - public static function from_consumer_and_token($consumer, $token, $http_method, $http_url, $parameters=NULL) { + public static function from_consumer_and_token($consumer, $token, $http_method, $http_url, $parameters = NULL) + { @$parameters or $parameters = array(); - $defaults = array("oauth_version" => OAuthRequest::$version, - "oauth_nonce" => OAuthRequest::generate_nonce(), - "oauth_timestamp" => OAuthRequest::generate_timestamp(), - "oauth_consumer_key" => $consumer->key); + $defaults = array( + "oauth_version" => OAuthRequest::$version, + "oauth_nonce" => OAuthRequest::generate_nonce(), + "oauth_timestamp" => OAuthRequest::generate_timestamp(), + "oauth_consumer_key" => $consumer->key + ); if ($token) $defaults['oauth_token'] = $token->key; @@ -315,7 +341,8 @@ class OAuthRequest { return new OAuthRequest($http_method, $http_url, $parameters); } - public function set_parameter($name, $value, $allow_duplicates = true) { + public function set_parameter($name, $value, $allow_duplicates = true) + { if ($allow_duplicates && isset($this->parameters[$name])) { // We have already added parameter(s) with this name, so add to the list if (is_scalar($this->parameters[$name])) { @@ -330,15 +357,18 @@ class OAuthRequest { } } - public function get_parameter($name) { + public function get_parameter($name) + { return isset($this->parameters[$name]) ? $this->parameters[$name] : null; } - public function get_parameters() { + public function get_parameters() + { return $this->parameters; } - public function unset_parameter($name) { + public function unset_parameter($name) + { unset($this->parameters[$name]); } @@ -346,7 +376,8 @@ class OAuthRequest { * The request parameters, sorted and concatenated into a normalized string. * @return string */ - public function get_signable_parameters() { + public function get_signable_parameters() + { // Grab all parameters $params = $this->parameters; @@ -366,7 +397,8 @@ class OAuthRequest { * and the parameters (normalized), each urlencoded * and the concated with &. */ - public function get_signature_base_string() { + public function get_signature_base_string() + { $parts = array( $this->get_normalized_http_method(), $this->get_normalized_http_url(), @@ -381,7 +413,8 @@ class OAuthRequest { /** * just uppercases the http method */ - public function get_normalized_http_method() { + public function get_normalized_http_method() + { return strtoupper($this->http_method); } @@ -389,7 +422,8 @@ class OAuthRequest { * parses the url and rebuilds it to be * scheme://host/path */ - public function get_normalized_http_url() { + public function get_normalized_http_url() + { $parts = parse_url($this->http_url); $port = @$parts['port']; @@ -400,7 +434,8 @@ class OAuthRequest { $port or $port = ($scheme == 'https') ? '443' : '80'; if (($scheme == 'https' && $port != '443') - || ($scheme == 'http' && $port != '80')) { + || ($scheme == 'http' && $port != '80') + ) { $host = "$host:$port"; } return "$scheme://$host$path"; @@ -409,11 +444,12 @@ class OAuthRequest { /** * builds a url usable for a GET request */ - public function to_url() { + public function to_url() + { $post_data = $this->to_postdata(); $out = $this->get_normalized_http_url(); if ($post_data) { - $out .= '?'.$post_data; + $out .= '?' . $post_data; } return $out; } @@ -421,9 +457,10 @@ class OAuthRequest { /** * builds the data one would send in a POST request */ - public function to_postdata($raw = false) { + public function to_postdata($raw = false) + { if ($raw) - return($this->parameters); + return $this->parameters; else return OAuthUtil::build_http_query($this->parameters); } @@ -431,15 +468,15 @@ class OAuthRequest { /** * builds the Authorization: header */ - public function to_header($realm=null) { + public function to_header($realm = null) + { $first = true; - if($realm) { + if ($realm) { $out = 'Authorization: OAuth realm="' . OAuthUtil::urlencode_rfc3986($realm) . '"'; $first = false; } else $out = 'Authorization: OAuth'; - $total = array(); foreach ($this->parameters as $k => $v) { if (substr($k, 0, 5) != "oauth") continue; if (is_array($v)) { @@ -447,20 +484,22 @@ class OAuthRequest { } $out .= ($first) ? ' ' : ','; $out .= OAuthUtil::urlencode_rfc3986($k) . - '="' . - OAuthUtil::urlencode_rfc3986($v) . - '"'; + '="' . + OAuthUtil::urlencode_rfc3986($v) . + '"'; $first = false; } return $out; } - public function __toString() { + public function __toString() + { return $this->to_url(); } - public function sign_request($signature_method, $consumer, $token) { + public function sign_request($signature_method, $consumer, $token) + { $this->set_parameter( "oauth_signature_method", $signature_method->get_name(), @@ -470,7 +509,8 @@ class OAuthRequest { $this->set_parameter("oauth_signature", $signature, false); } - public function build_signature($signature_method, $consumer, $token) { + public function build_signature($signature_method, $consumer, $token) + { $signature = $signature_method->build_signature($this, $consumer, $token); return $signature; } @@ -478,33 +518,35 @@ class OAuthRequest { /** * util function: current timestamp */ - private static function generate_timestamp() { + private static function generate_timestamp() + { return time(); } /** * util function: current nonce */ - private static function generate_nonce() { - $mt = microtime(); - $rand = mt_rand(); - - return md5($mt . $rand); // md5s look nicer than numbers + private static function generate_nonce() + { + return bin2hex(random_bytes(16)); } } -class OAuthServer { +class OAuthServer +{ protected $timestamp_threshold = 300; // in seconds, five minutes protected $version = '1.0'; // hi blaine protected $signature_methods = array(); protected $data_store; - function __construct($data_store) { + function __construct($data_store) + { $this->data_store = $data_store; } - public function add_signature_method($signature_method) { + public function add_signature_method($signature_method) + { $this->signature_methods[$signature_method->get_name()] = $signature_method; } @@ -515,7 +557,8 @@ class OAuthServer { * process a request_token request * returns the request token on success */ - public function fetch_request_token(&$request) { + public function fetch_request_token(&$request) + { $this->get_version($request); $consumer = $this->get_consumer($request); @@ -536,7 +579,8 @@ class OAuthServer { * process an access_token request * returns the access token on success */ - public function fetch_access_token(&$request) { + public function fetch_access_token(&$request) + { $this->get_version($request); $consumer = $this->get_consumer($request); @@ -556,22 +600,24 @@ class OAuthServer { /** * verify an api call, checks all the parameters */ - public function verify_request(&$request) { + public function verify_request(&$request) + { $this->get_version($request); $consumer = $this->get_consumer($request); $token = $this->get_token($request, $consumer, "access"); $this->check_signature($request, $consumer, $token); - return array($consumer, $token); + return [$consumer, $token]; } // Internals from here /** * version 1 */ - private function get_version(&$request) { + private function get_version(&$request) + { $version = $request->get_parameter("oauth_version"); if (!$version) { - // Service Providers MUST assume the protocol version to be 1.0 if this parameter is not present. + // Service Providers MUST assume the protocol version to be 1.0 if this parameter is not present. // Chapter 7.0 ("Accessing Protected Ressources") $version = '1.0'; } @@ -584,9 +630,10 @@ class OAuthServer { /** * figure out the signature with some defaults */ - private function get_signature_method(&$request) { + private function get_signature_method(&$request) + { $signature_method = - @$request->get_parameter("oauth_signature_method"); + @$request->get_parameter("oauth_signature_method"); if (!$signature_method) { // According to chapter 7 ("Accessing Protected Ressources") the signature-method @@ -594,12 +641,14 @@ class OAuthServer { throw new OAuthException('No signature method parameter. This parameter is required'); } - if (!in_array($signature_method, - array_keys($this->signature_methods))) { + if (!in_array( + $signature_method, + array_keys($this->signature_methods) + )) { throw new OAuthException( "Signature method '$signature_method' not supported " . - "try one of the following: " . - implode(", ", array_keys($this->signature_methods)) + "try one of the following: " . + implode(", ", array_keys($this->signature_methods)) ); } return $this->signature_methods[$signature_method]; @@ -608,7 +657,8 @@ class OAuthServer { /** * try to find the consumer for the provided request's consumer key */ - private function get_consumer(&$request) { + private function get_consumer(&$request) + { $consumer_key = @$request->get_parameter("oauth_consumer_key"); if (!$consumer_key) { throw new OAuthException("Invalid consumer key"); @@ -625,10 +675,13 @@ class OAuthServer { /** * try to find the token for the provided request's token key */ - private function get_token(&$request, $consumer, $token_type="access") { + private function get_token(&$request, $consumer, $token_type = "access") + { $token_field = @$request->get_parameter('oauth_token'); $token = $this->data_store->lookup_token( - $consumer, $token_type, $token_field + $consumer, + $token_type, + $token_field ); if (!$token) { throw new OAuthException("Invalid $token_type token: $token_field"); @@ -640,7 +693,8 @@ class OAuthServer { * all-in-one function to check the signature on a request * should guess the signature method appropriately */ - private function check_signature(&$request, $consumer, $token) { + private function check_signature(&$request, $consumer, $token) + { // this should probably be in a different method $timestamp = @$request->get_parameter('oauth_timestamp'); $nonce = @$request->get_parameter('oauth_nonce'); @@ -657,7 +711,7 @@ class OAuthServer { $token, $signature ); - + if (!$valid_sig) { throw new OAuthException("Invalid signature"); @@ -667,12 +721,13 @@ class OAuthServer { /** * check that the timestamp is new enough */ - private function check_timestamp($timestamp) { - if( ! $timestamp ) + private function check_timestamp($timestamp) + { + if (!$timestamp) throw new OAuthException( 'Missing timestamp parameter. The parameter is required' ); - + // verify that timestamp is recentish $now = time(); if (abs($now - $timestamp) > $this->timestamp_threshold) { @@ -685,8 +740,9 @@ class OAuthServer { /** * check that the nonce is not repeated */ - private function check_nonce($consumer, $token, $nonce, $timestamp) { - if( ! $nonce ) + private function check_nonce($consumer, $token, $nonce, $timestamp) + { + if (!$nonce) throw new OAuthException( 'Missing nonce parameter. The parameter is required' ); @@ -702,65 +758,73 @@ class OAuthServer { throw new OAuthException("Nonce already used: $nonce"); } } - } -class OAuthDataStore { - function lookup_consumer($consumer_key) { +class OAuthDataStore +{ + function lookup_consumer($consumer_key) + { // implement me } - function lookup_token($consumer, $token_type, $token) { + function lookup_token($consumer, $token_type, $token) + { // implement me } - function lookup_nonce($consumer, $token, $nonce, $timestamp) { + function lookup_nonce($consumer, $token, $nonce, $timestamp) + { // implement me } - function new_request_token($consumer, $callback = null) { + function new_request_token($consumer, $callback = null) + { // return a new token attached to this consumer } - function new_access_token($token, $consumer, $verifier = null) { + function new_access_token($token, $consumer, $verifier = null) + { // return a new access token attached to this consumer // for the user associated with this token if the request token // is authorized // should also invalidate the request token } - } -class OAuthUtil { - public static function urlencode_rfc3986($input) { - if (is_array($input)) { - return array_map(array('OAuthUtil', 'urlencode_rfc3986'), $input); - } else if (is_scalar($input)) { - return str_replace( - '+', - ' ', - str_replace('%7E', '~', rawurlencode($input)) - ); - } else { - return ''; +class OAuthUtil +{ + public static function urlencode_rfc3986($input) + { + if (is_array($input)) { + return array_map(['OAuthUtil', 'urlencode_rfc3986'], $input); + } else if (is_scalar($input)) { + return str_replace( + '+', + ' ', + str_replace('%7E', '~', rawurlencode($input)) + ); + } else { + return ''; + } } -} // This decode function isn't taking into consideration the above // modifications to the encoding process. However, this method doesn't // seem to be used anywhere so leaving it as is. - public static function urldecode_rfc3986($string) { + public static function urldecode_rfc3986($string) + { return urldecode($string); } // Utility function for turning the Authorization: header into // parameters, has to do some unescaping // Can filter out any non-oauth parameters if needed (default behaviour) - public static function split_header($header, $only_allow_oauth_parameters = true) { + public static function split_header($header, $only_allow_oauth_parameters = true) + { $pattern = '/(([-_a-z]*)=("([^"]*)"|([^,]*)),?)/'; $offset = 0; - $params = array(); + $params = []; while (preg_match($pattern, $header, $matches, PREG_OFFSET_CAPTURE, $offset) > 0) { $match = $matches[0]; $header_name = $matches[2][0]; @@ -779,7 +843,8 @@ class OAuthUtil { } // helper to try to sort out headers for people who aren't running apache - public static function get_headers() { + public static function get_headers() + { if (function_exists('apache_request_headers')) { // we need this to get the actual Authorization: header // because apache tends to tell us it doesn't exist @@ -789,22 +854,22 @@ class OAuthUtil { // we always want the keys to be Cased-Like-This and arh() // returns the headers in the same case as they are in the // request - $out = array(); - foreach( $headers AS $key => $value ) { + $out = []; + foreach ($headers as $key => $value) { $key = str_replace( - " ", - "-", - ucwords(strtolower(str_replace("-", " ", $key))) - ); + " ", + "-", + ucwords(strtolower(str_replace("-", " ", $key))) + ); $out[$key] = $value; } } else { // otherwise we don't have apache and are just going to have to hope // that $_SERVER actually contains what we need - $out = array(); - if( isset($_SERVER['CONTENT_TYPE']) ) + $out = []; + if (isset($_SERVER['CONTENT_TYPE'])) $out['Content-Type'] = $_SERVER['CONTENT_TYPE']; - if( isset($_ENV['CONTENT_TYPE']) ) + if (isset($_ENV['CONTENT_TYPE'])) $out['Content-Type'] = $_ENV['CONTENT_TYPE']; foreach ($_SERVER as $key => $value) { @@ -827,12 +892,13 @@ class OAuthUtil { // This function takes a input like a=b&a=c&d=e and returns the parsed // parameters like this // array('a' => array('b','c'), 'd' => 'e') - public static function parse_parameters( $input ) { + public static function parse_parameters($input) + { if (!isset($input) || !$input) return array(); $pairs = explode('&', $input); - $parsed_parameters = array(); + $parsed_parameters = []; foreach ($pairs as $pair) { $split = explode('=', $pair, 2); $parameter = OAuthUtil::urldecode_rfc3986($split[0]); @@ -845,7 +911,7 @@ class OAuthUtil { if (is_scalar($parsed_parameters[$parameter])) { // This is the first duplicate, so transform scalar (string) into an array // so we can add the duplicates - $parsed_parameters[$parameter] = array($parsed_parameters[$parameter]); + $parsed_parameters[$parameter] = [$parsed_parameters[$parameter]]; } $parsed_parameters[$parameter][] = $value; @@ -856,7 +922,8 @@ class OAuthUtil { return $parsed_parameters; } - public static function build_http_query($params) { + public static function build_http_query($params) + { if (!$params) return ''; // Urlencode both keys and values @@ -868,7 +935,7 @@ class OAuthUtil { // Ref: Spec: 9.1.1 (1) uksort($params, 'strcmp'); - $pairs = array(); + $pairs = []; foreach ($params as $parameter => $value) { if (is_array($value)) { // If two or more parameters share the same name, they are sorted by their value @@ -886,5 +953,3 @@ class OAuthUtil { return implode('&', $pairs); } } - -?> diff --git a/mod/lostpass.php b/mod/lostpass.php index 01e84268b..ecab0982c 100644 --- a/mod/lostpass.php +++ b/mod/lostpass.php @@ -1,4 +1,5 @@ internalRedirect(); } - $pwdreset_token = Strings::getRandomName(12) . mt_rand(1000, 9999); + $pwdreset_token = Strings::getRandomName(12) . random_int(1000, 9999); $fields = [ 'pwdreset' => $pwdreset_token, diff --git a/src/Core/Authentication.php b/src/Core/Authentication.php index 59061c04c..fd34dbc88 100644 --- a/src/Core/Authentication.php +++ b/src/Core/Authentication.php @@ -1,4 +1,5 @@ $user["uid"], + $value = json_encode([ + "uid" => $user["uid"], "hash" => self::getCookieHashForUser($user), - "ip" => defaults($_SERVER, 'REMOTE_ADDR', '0.0.0.0')]); + "ip" => defaults($_SERVER, 'REMOTE_ADDR', '0.0.0.0') + ]); } else { $value = ""; } @@ -88,4 +93,3 @@ class Authentication extends BaseObject } } } - diff --git a/src/Model/Register.php b/src/Model/Register.php index 8a03f379a..9f6d36974 100644 --- a/src/Model/Register.php +++ b/src/Model/Register.php @@ -3,6 +3,7 @@ /** * @file src/Model/Register.php */ + namespace Friendica\Model; use Friendica\Database\DBA; @@ -83,7 +84,7 @@ class Register */ public static function createForInvitation() { - $code = Strings::getRandomName(8) . srand(1000, 9999); + $code = Strings::getRandomName(8) . random_int(1000, 9999); $fields = [ 'hash' => $code, diff --git a/src/Model/User.php b/src/Model/User.php index 141ecf059..b6121ad04 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -1,8 +1,10 @@ $user_info, 'blocked' => 0, @@ -374,9 +381,11 @@ class User ); } else { $fields = ['uid', 'password', 'legacy_password']; - $condition = ["(`email` = ? OR `username` = ? OR `nickname` = ?) + $condition = [ + "(`email` = ? OR `username` = ? OR `nickname` = ?) AND NOT `blocked` AND NOT `account_expired` AND NOT `account_removed` AND `verified`", - $user_info, $user_info, $user_info]; + $user_info, $user_info, $user_info + ]; $user = DBA::selectFirst('user', $fields, $condition); } @@ -395,7 +404,7 @@ class User */ public static function generateNewPassword() { - return ucfirst(Strings::getRandomName(8)) . mt_rand(1000, 9999); + return ucfirst(Strings::getRandomName(8)) . random_int(1000, 9999); } /** @@ -671,7 +680,8 @@ class User } // Check existing and deleted accounts for this nickname. - if (DBA::exists('user', ['nickname' => $nickname]) + if ( + DBA::exists('user', ['nickname' => $nickname]) || DBA::exists('userd', ['username' => $nickname]) ) { throw new Exception(L10n::t('Nickname is already registered. Please choose another.')); @@ -839,7 +849,8 @@ class User */ public static function sendRegisterPendingEmail($user, $sitename, $siteurl, $password) { - $body = Strings::deindent(L10n::t(' + $body = Strings::deindent(L10n::t( + ' Dear %1$s, Thank you for registering at %2$s. Your account is pending for approval by the administrator. @@ -849,7 +860,11 @@ class User Login Name: %4$s Password: %5$s ', - $user['username'], $sitename, $siteurl, $user['nickname'], $password + $user['username'], + $sitename, + $siteurl, + $user['nickname'], + $password )); return notification([ @@ -875,13 +890,16 @@ class User */ public static function sendRegisterOpenEmail($user, $sitename, $siteurl, $password) { - $preamble = Strings::deindent(L10n::t(' - Dear %1$s, + $preamble = Strings::deindent(L10n::t( + ' + Dear %1$s, Thank you for registering at %2$s. Your account has been created. - ', - $user['username'], $sitename + ', + $user['username'], + $sitename )); - $body = Strings::deindent(L10n::t(' + $body = Strings::deindent(L10n::t( + ' The login details are as follows: Site Location: %3$s @@ -908,7 +926,11 @@ class User If you ever want to delete your account, you can do so at %3$s/removeme Thank you and welcome to %2$s.', - $user['nickname'], $sitename, $siteurl, $user['username'], $password + $user['nickname'], + $sitename, + $siteurl, + $user['username'], + $password )); return notification([ @@ -989,33 +1011,45 @@ class User if ($user['parent-uid'] == 0) { // First add our own entry - $identities = [['uid' => $user['uid'], + $identities = [[ + 'uid' => $user['uid'], 'username' => $user['username'], - 'nickname' => $user['nickname']]]; + 'nickname' => $user['nickname'] + ]]; // Then add all the children - $r = DBA::select('user', ['uid', 'username', 'nickname'], - ['parent-uid' => $user['uid'], 'account_removed' => false]); + $r = DBA::select( + 'user', + ['uid', 'username', 'nickname'], + ['parent-uid' => $user['uid'], 'account_removed' => false] + ); if (DBA::isResult($r)) { $identities = array_merge($identities, DBA::toArray($r)); } } else { // First entry is our parent - $r = DBA::select('user', ['uid', 'username', 'nickname'], - ['uid' => $user['parent-uid'], 'account_removed' => false]); + $r = DBA::select( + 'user', + ['uid', 'username', 'nickname'], + ['uid' => $user['parent-uid'], 'account_removed' => false] + ); if (DBA::isResult($r)) { $identities = DBA::toArray($r); } // Then add all siblings - $r = DBA::select('user', ['uid', 'username', 'nickname'], - ['parent-uid' => $user['parent-uid'], 'account_removed' => false]); + $r = DBA::select( + 'user', + ['uid', 'username', 'nickname'], + ['parent-uid' => $user['parent-uid'], 'account_removed' => false] + ); if (DBA::isResult($r)) { $identities = array_merge($identities, DBA::toArray($r)); } } - $r = DBA::p("SELECT `user`.`uid`, `user`.`username`, `user`.`nickname` + $r = DBA::p( + "SELECT `user`.`uid`, `user`.`username`, `user`.`nickname` FROM `manage` INNER JOIN `user` ON `manage`.`mid` = `user`.`uid` WHERE `user`.`account_removed` = 0 AND `manage`.`uid` = ?", @@ -1061,13 +1095,13 @@ class User while ($user = DBA::fetch($userStmt)) { $statistics['total_users']++; - if ((strtotime($user['login_date']) > $halfyear) || - (strtotime($user['last-item']) > $halfyear)) { + if ((strtotime($user['login_date']) > $halfyear) || (strtotime($user['last-item']) > $halfyear) + ) { $statistics['active_users_halfyear']++; } - if ((strtotime($user['login_date']) > $month) || - (strtotime($user['last-item']) > $month)) { + if ((strtotime($user['login_date']) > $month) || (strtotime($user['last-item']) > $month) + ) { $statistics['active_users_monthly']++; } } diff --git a/src/Module/Login.php b/src/Module/Login.php index 40e376aeb..a8c9f9fb9 100644 --- a/src/Module/Login.php +++ b/src/Module/Login.php @@ -1,7 +1,9 @@ User::getIdFromPasswordAuthentication($username, $password)] ); } @@ -176,7 +178,9 @@ class Login extends BaseModule $data = json_decode($_COOKIE["Friendica"]); if (isset($data->uid)) { - $user = DBA::selectFirst('user', [], + $user = DBA::selectFirst( + 'user', + [], [ 'uid' => $data->uid, 'blocked' => false, @@ -186,7 +190,13 @@ class Login extends BaseModule ] ); if (DBA::isResult($user)) { - if ($data->hash != Authentication::getCookieHashForUser($user)) { + // Time safe comparision of the two hashes. + $validSession = hash_equals( + Authentication::getCookieHashForUser($user), + $data->hash + ); + + if (!$validSession) { Logger::log("Hash for user " . $data->uid . " doesn't fit."); Authentication::deleteSession(); $a->internalRedirect(); @@ -229,7 +239,9 @@ class Login extends BaseModule $a->internalRedirect(); } - $user = DBA::selectFirst('user', [], + $user = DBA::selectFirst( + 'user', + [], [ 'uid' => $_SESSION['uid'], 'blocked' => false, @@ -312,12 +324,12 @@ class Login extends BaseModule '$logout' => L10n::t('Logout'), '$login' => L10n::t('Login'), - '$lname' => ['username', L10n::t('Nickname or Email: ') , '', ''], + '$lname' => ['username', L10n::t('Nickname or Email: '), '', ''], '$lpassword' => ['password', L10n::t('Password: '), '', ''], '$lremember' => ['remember', L10n::t('Remember me'), 0, ''], '$openid' => !$noid, - '$lopenid' => ['openid_url', L10n::t('Or login using OpenID: '),'',''], + '$lopenid' => ['openid_url', L10n::t('Or login using OpenID: '), '', ''], '$hiddens' => $hiddens, diff --git a/src/Network/FKOAuthDataStore.php b/src/Network/FKOAuthDataStore.php index d1f43172b..9fba89685 100644 --- a/src/Network/FKOAuthDataStore.php +++ b/src/Network/FKOAuthDataStore.php @@ -29,7 +29,7 @@ class FKOAuthDataStore extends OAuthDataStore */ private static function genToken() { - return md5(base64_encode(pack('N6', mt_rand(), mt_rand(), mt_rand(), mt_rand(), mt_rand(), uniqid()))); + return bin2hex(random_bytes(16)); } /** @@ -119,7 +119,8 @@ class FKOAuthDataStore extends OAuthDataStore 'secret' => $sec, 'client_id' => $k, 'scope' => 'request', - 'expires' => time() + REQUEST_TOKEN_DURATION] + 'expires' => time() + REQUEST_TOKEN_DURATION + ] ); if (!$r) { @@ -162,7 +163,8 @@ class FKOAuthDataStore extends OAuthDataStore 'client_id' => $consumer->key, 'scope' => 'access', 'expires' => time() + ACCESS_TOKEN_DURATION, - 'uid' => $uverifier] + 'uid' => $uverifier + ] ); if ($r) { diff --git a/src/Util/Strings.php b/src/Util/Strings.php index 88dd1d39f..cd065aed1 100644 --- a/src/Util/Strings.php +++ b/src/Util/Strings.php @@ -1,4 +1,5 @@ ' . ContactSelector::networkToName($network, $url) . ''; + $network_name = '' . ContactSelector::networkToName($network, $url) . ''; } else { $network_name = ContactSelector::networkToName($network); } @@ -176,11 +180,11 @@ class Strings /** * @brief Remove indentation from a text - * + * * @param string $text String to be transformed. * @param string $chr Optional. Indentation tag. Default tab (\t). * @param int $count Optional. Default null. - * + * * @return string Transformed string. */ public static function deindent($text, $chr = "[\t ]", $count = NULL) @@ -206,10 +210,10 @@ class Strings /** * @brief Get byte size returned in a Data Measurement (KB, MB, GB) - * + * * @param int $bytes The number of bytes to be measured * @param int $precision Optional. Default 2. - * + * * @return string Size with measured units. */ public static function formatBytes($bytes, $precision = 2) @@ -225,9 +229,9 @@ class Strings /** * @brief Protect percent characters in sprintf calls - * + * * @param string $s String to transform. - * + * * @return string Transformed string. */ public static function protectSprintf($s) @@ -237,10 +241,10 @@ class Strings /** * @brief Base64 Encode URL and translate +/ to -_ Optionally strip padding. - * + * * @param string $s URL to encode * @param boolean $strip_padding Optional. Default false - * + * * @return string Encoded URL */ public static function base64UrlEncode($s, $strip_padding = false) @@ -254,13 +258,13 @@ class Strings return $s; } - /** - * @brief Decode Base64 Encoded URL and translate -_ to +/ - * @param string $s URL to decode - * - * @return string Decoded URL - * @throws \Exception - */ + /** + * @brief Decode Base64 Encoded URL and translate -_ to +/ + * @param string $s URL to decode + * + * @return string Decoded URL + * @throws \Exception + */ public static function base64UrlDecode($s) { if (is_array($s)) { @@ -291,7 +295,7 @@ class Strings * @brief Normalize url * * @param string $url URL to be normalized. - * + * * @return string Normalized URL. */ public static function normaliseLink($url) @@ -302,9 +306,9 @@ class Strings /** * @brief Normalize OpenID identity - * + * * @param string $s OpenID Identity - * + * * @return string normalized OpenId Identity */ public static function normaliseOpenID($s) @@ -329,51 +333,51 @@ class Strings } - /** - * Ensures the provided URI has its query string punctuation in order. - * - * @param string $uri - * @return string - */ - public static function ensureQueryParameter($uri) - { - if (strpos($uri, '?') === false && ($pos = strpos($uri, '&')) !== false) { - $uri = substr($uri, 0, $pos) . '?' . substr($uri, $pos + 1); - } + /** + * Ensures the provided URI has its query string punctuation in order. + * + * @param string $uri + * @return string + */ + public static function ensureQueryParameter($uri) + { + if (strpos($uri, '?') === false && ($pos = strpos($uri, '&')) !== false) { + $uri = substr($uri, 0, $pos) . '?' . substr($uri, $pos + 1); + } - return $uri; - } + return $uri; + } - /** - * Check if the trimmed provided string is starting with one of the provided characters - * - * @param string $string - * @param array $chars - * @return bool - */ - public static function startsWith($string, array $chars) - { - $return = in_array(substr(trim($string), 0, 1), $chars); + /** + * Check if the trimmed provided string is starting with one of the provided characters + * + * @param string $string + * @param array $chars + * @return bool + */ + public static function startsWith($string, array $chars) + { + $return = in_array(substr(trim($string), 0, 1), $chars); - return $return; - } + return $return; + } - /** - * Returns the regular expression string to match URLs in a given text - * - * @return string - * @see https://daringfireball.net/2010/07/improved_regex_for_matching_urls - */ - public static function autoLinkRegEx() - { - return '@ + /** + * Returns the regular expression string to match URLs in a given text + * + * @return string + * @see https://daringfireball.net/2010/07/improved_regex_for_matching_urls + */ + public static function autoLinkRegEx() + { + return '@ (??«»“”‘’.] # Domain can\'t start with a . + [^/\s\xA0`!()\[\]{};:\'",<>?«»“”‘’.] # Domain can\'t start with a . [^/\s\xA0`!()\[\]{};:\'",<>?«»“”‘’]+ # Domain can\'t end with a . \. [^/\s\xA0`!()\[\]{};:\'".,<>?«»“”‘’]+/? # Followed by a slash @@ -386,21 +390,21 @@ class Strings [^\s\xA0`!()\[\]{};:\'".,<>?«»“”‘’] # not a space or one of these punct chars )* )@xiu'; - } + } - /** - * Ensures a single path item doesn't contain any path-traversing characters - * - * @see https://stackoverflow.com/a/46097713 - * @param string $pathItem - * @return string - */ - public static function sanitizeFilePathItem($pathItem) - { - $pathItem = str_replace('/', '_', $pathItem); - $pathItem = str_replace('\\', '_', $pathItem); - $pathItem = str_replace(DIRECTORY_SEPARATOR, '_', $pathItem); // In case it does not equal the standard values + /** + * Ensures a single path item doesn't contain any path-traversing characters + * + * @see https://stackoverflow.com/a/46097713 + * @param string $pathItem + * @return string + */ + public static function sanitizeFilePathItem($pathItem) + { + $pathItem = str_replace('/', '_', $pathItem); + $pathItem = str_replace('\\', '_', $pathItem); + $pathItem = str_replace(DIRECTORY_SEPARATOR, '_', $pathItem); // In case it does not equal the standard values - return $pathItem; - } + return $pathItem; + } } From dc01bdbc80cd382e33f61a21ce836a1e44a6c44c Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Thu, 10 Oct 2019 20:43:32 -0800 Subject: [PATCH 2/6] Use the utility instead. --- library/OAuth1.php | 2 +- src/Network/FKOAuthDataStore.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/OAuth1.php b/library/OAuth1.php index 723f6592e..e3ca24646 100644 --- a/library/OAuth1.php +++ b/library/OAuth1.php @@ -528,7 +528,7 @@ class OAuthRequest */ private static function generate_nonce() { - return bin2hex(random_bytes(16)); + return Friendica\Util\Strings::getRandomHex(32); } } diff --git a/src/Network/FKOAuthDataStore.php b/src/Network/FKOAuthDataStore.php index 9fba89685..e3ee68d9c 100644 --- a/src/Network/FKOAuthDataStore.php +++ b/src/Network/FKOAuthDataStore.php @@ -29,7 +29,7 @@ class FKOAuthDataStore extends OAuthDataStore */ private static function genToken() { - return bin2hex(random_bytes(16)); + return Friendica\Util\Strings::getRandomHex(32); } /** From 3940e804e3ee4ac921e109f62a73fac2becaa611 Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Thu, 10 Oct 2019 20:48:13 -0800 Subject: [PATCH 3/6] Remove uneeded variable. --- src/Module/Login.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Module/Login.php b/src/Module/Login.php index a8c9f9fb9..8affd7755 100644 --- a/src/Module/Login.php +++ b/src/Module/Login.php @@ -190,13 +190,10 @@ class Login extends BaseModule ] ); if (DBA::isResult($user)) { - // Time safe comparision of the two hashes. - $validSession = hash_equals( + if (!hash_equals( Authentication::getCookieHashForUser($user), $data->hash - ); - - if (!$validSession) { + )) { Logger::log("Hash for user " . $data->uid . " doesn't fit."); Authentication::deleteSession(); $a->internalRedirect(); From b5dac16defccaf5a929368b2e7ca6e8c15e40984 Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Thu, 10 Oct 2019 20:50:51 -0800 Subject: [PATCH 4/6] Comply with coding style. --- src/Util/Strings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Util/Strings.php b/src/Util/Strings.php index cd065aed1..ef7a2df06 100644 --- a/src/Util/Strings.php +++ b/src/Util/Strings.php @@ -68,7 +68,7 @@ class Strings */ public static function escapeHtml($string) { - return htmlentities($string, ENT_QUOTES | ENT_HTML5, "UTF-8", false); + return htmlentities($string, ENT_QUOTES | ENT_HTML5, 'UTF-8', false); } /** From 1d1c089f84b05312d594ecd052c9a0d8aa942f76 Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Thu, 10 Oct 2019 21:21:22 -0800 Subject: [PATCH 5/6] Make test comply. --- tests/src/Util/StringsTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/src/Util/StringsTest.php b/tests/src/Util/StringsTest.php index f92618310..03bc88d74 100644 --- a/tests/src/Util/StringsTest.php +++ b/tests/src/Util/StringsTest.php @@ -12,7 +12,7 @@ use PHPUnit\Framework\TestCase; */ class StringsTest extends TestCase { - /** + /** * randomnames should be random, even length */ public function testRandomEven() @@ -64,9 +64,9 @@ class StringsTest extends TestCase $randomname2 = Strings::getRandomName(1); $this->assertEquals(1, strlen($randomname2)); - } - - /** + } + + /** * test, that tags are escaped */ public function testEscapeHtml() @@ -78,7 +78,7 @@ class StringsTest extends TestCase $this->assertEquals('[submit type="button" onclick="alert(\'failed!\');" /]', $validstring); $this->assertEquals( - "<submit type="button" onclick="alert('failed!');" />", + '<submit type="button" onclick="alert('failed!');" />', $escapedString ); } From e1e1d26b5beb166af6f747809fbd67eca499ae06 Mon Sep 17 00:00:00 2001 From: dew-git <55564947+dew-git@users.noreply.github.com> Date: Fri, 11 Oct 2019 00:00:15 -0800 Subject: [PATCH 6/6] Revert random_int changes. --- src/Util/Strings.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Util/Strings.php b/src/Util/Strings.php index ef7a2df06..91b86fbfd 100644 --- a/src/Util/Strings.php +++ b/src/Util/Strings.php @@ -86,7 +86,7 @@ class Strings $vowels = ['a', 'a', 'ai', 'au', 'e', 'e', 'e', 'ee', 'ea', 'i', 'ie', 'o', 'ou', 'u']; - if (random_int(0, 5) == 4) { + if (mt_rand(0, 5) == 4) { $vowels[] = 'y'; } @@ -123,7 +123,7 @@ class Strings 'kh', 'kl', 'kr', 'mn', 'pl', 'pr', 'rh', 'tr', 'qu', 'wh', 'q' ]; - $start = random_int(0, 2); + $start = mt_rand(0, 2); if ($start == 0) { $table = $vowels; } else { @@ -133,7 +133,7 @@ class Strings $word = ''; for ($x = 0; $x < $len; $x++) { - $r = random_int(0, count($table) - 1); + $r = mt_rand(0, count($table) - 1); $word .= $table[$r]; if ($table == $vowels) {