Fix security vulnerbilities.

Fix possible length extension attack, predicable generators, timing attacks on hash comparision and improved formatting.
This commit is contained in:
dew-git 2019-10-10 15:21:41 -08:00
parent 50fa119f79
commit 811cdcdfcb
8 changed files with 455 additions and 332 deletions

View file

@ -1,4 +1,5 @@
<?php
/**
* @file /src/Core/Authentication.php
*/
@ -10,8 +11,8 @@ use Friendica\BaseObject;
use Friendica\Network\HTTPException\ForbiddenException;
/**
* Handle Authentification, Session and Cookies
*/
* Handle Authentification, Session and Cookies
*/
class Authentication extends BaseObject
{
/**
@ -24,9 +25,11 @@ class Authentication extends BaseObject
*/
public static function getCookieHashForUser($user)
{
return(hash("sha256", Config::get("system", "site_prvkey") .
$user["prvkey"] .
$user["password"]));
return hash_hmac(
"sha256",
hash_hmac("sha256", $user["password"], $user["privkey"]),
Config::get("system", "site_prvkey")
);
}
/**
@ -43,9 +46,11 @@ class Authentication extends BaseObject
}
if ($user) {
$value = json_encode(["uid" => $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
}
}
}

View file

@ -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,

View file

@ -1,8 +1,10 @@
<?php
/**
* @file src/Model/User.php
* @brief This file includes the User class with user related database functions
*/
namespace Friendica\Model;
use DivineOmega\PasswordExposed;
@ -155,8 +157,10 @@ class User
* @return boolean|array
* @throws Exception
*/
public static function getOwnerDataById($uid, $check_valid = true) {
$r = DBA::fetchFirst("SELECT
public static function getOwnerDataById($uid, $check_valid = true)
{
$r = DBA::fetchFirst(
"SELECT
`contact`.*,
`user`.`prvkey` AS `uprvkey`,
`user`.`timezone`,
@ -355,7 +359,8 @@ class User
$user = $user_info;
}
if (!isset($user['uid'])
if (
!isset($user['uid'])
|| !isset($user['password'])
|| !isset($user['legacy_password'])
) {
@ -363,7 +368,9 @@ class User
}
} elseif (is_int($user_info) || is_string($user_info)) {
if (is_int($user_info)) {
$user = DBA::selectFirst('user', ['uid', 'password', 'legacy_password'],
$user = DBA::selectFirst(
'user',
['uid', 'password', 'legacy_password'],
[
'uid' => $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']++;
}
}

View file

@ -1,7 +1,9 @@
<?php
/**
* @file src/Module/Login.php
*/
namespace Friendica\Module;
use Exception;
@ -48,10 +50,8 @@ class Login extends BaseModule
// OpenId Login
if (
empty($_POST['password'])
&& (
!empty($_POST['openid_url'])
|| !empty($_POST['username'])
)
&& (!empty($_POST['openid_url'])
|| !empty($_POST['username']))
) {
$openid_url = trim(defaults($_POST, 'openid_url', $_POST['username']));
@ -136,7 +136,9 @@ class Login extends BaseModule
throw new Exception(L10n::t('Login failed.'));
}
} else {
$record = DBA::selectFirst('user', [],
$record = DBA::selectFirst(
'user',
[],
['uid' => 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,

View file

@ -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) {

View file

@ -1,4 +1,5 @@
<?php
/**
* @file src/Util/Strings.php
*/
@ -13,13 +14,13 @@ use Friendica\Core\Logger;
*/
class Strings
{
/**
* @brief Generates a pseudo-random string of hexadecimal characters
*
* @param int $size
* @return string
* @throws \Exception
*/
/**
* @brief Generates a pseudo-random string of hexadecimal characters
*
* @param int $size
* @return string
* @throws \Exception
*/
public static function getRandomHex($size = 64)
{
$byte_size = ceil($size / 2);
@ -31,16 +32,16 @@ class Strings
return $return;
}
/**
* Checks, if the given string is a valid hexadecimal code
*
* @param string $hexCode
*
* @return bool
*/
/**
* Checks, if the given string is a valid hexadecimal code
*
* @param string $hexCode
*
* @return bool
*/
public static function isHex($hexCode)
{
return !empty($hexCode) ? @preg_match("/^[a-f0-9]{2,}$/i", $hexCode) && !(strlen($hexCode) & 1) : false;
return !empty($hexCode) ? @preg_match("/^[a-f0-9]{2,}$/i", $hexCode) && !(strlen($hexCode) & 1) : false;
}
/**
@ -62,19 +63,19 @@ class Strings
* @brief Use this on "body" or "content" input where angle chars shouldn't be removed,
* and allow them to be safely displayed.
* @param string $string
*
*
* @return string
*/
public static function escapeHtml($string)
{
return htmlspecialchars($string, ENT_COMPAT, 'UTF-8', false);
return htmlentities($string, ENT_QUOTES | ENT_HTML5, "UTF-8", false);
}
/**
* @brief Generate a string that's random, but usually pronounceable. Used to generate initial passwords
*
*
* @param int $len length
*
*
* @return string
*/
public static function getRandomName($len)
@ -85,40 +86,44 @@ class Strings
$vowels = ['a', 'a', 'ai', 'au', 'e', 'e', 'e', 'ee', 'ea', 'i', 'ie', 'o', 'ou', 'u'];
if (mt_rand(0, 5) == 4) {
if (random_int(0, 5) == 4) {
$vowels[] = 'y';
}
$cons = [
'b', 'bl', 'br',
'c', 'ch', 'cl', 'cr',
'd', 'dr',
'f', 'fl', 'fr',
'g', 'gh', 'gl', 'gr',
'h',
'j',
'k', 'kh', 'kl', 'kr',
'l',
'm',
'n',
'p', 'ph', 'pl', 'pr',
'qu',
'r', 'rh',
's' ,'sc', 'sh', 'sm', 'sp', 'st',
't', 'th', 'tr',
'v',
'w', 'wh',
'x',
'z', 'zh'
];
'b', 'bl', 'br',
'c', 'ch', 'cl', 'cr',
'd', 'dr',
'f', 'fl', 'fr',
'g', 'gh', 'gl', 'gr',
'h',
'j',
'k', 'kh', 'kl', 'kr',
'l',
'm',
'n',
'p', 'ph', 'pl', 'pr',
'qu',
'r', 'rh',
's', 'sc', 'sh', 'sm', 'sp', 'st',
't', 'th', 'tr',
'v',
'w', 'wh',
'x',
'z', 'zh'
];
$midcons = ['ck', 'ct', 'gn', 'ld', 'lf', 'lm', 'lt', 'mb', 'mm', 'mn', 'mp',
'nd', 'ng', 'nk', 'nt', 'rn', 'rp', 'rt'];
$midcons = [
'ck', 'ct', 'gn', 'ld', 'lf', 'lm', 'lt', 'mb', 'mm', 'mn', 'mp',
'nd', 'ng', 'nk', 'nt', 'rn', 'rp', 'rt'
];
$noend = ['bl', 'br', 'cl', 'cr', 'dr', 'fl', 'fr', 'gl', 'gr',
'kh', 'kl', 'kr', 'mn', 'pl', 'pr', 'rh', 'tr', 'qu', 'wh', 'q'];
$noend = [
'bl', 'br', 'cl', 'cr', 'dr', 'fl', 'fr', 'gl', 'gr',
'kh', 'kl', 'kr', 'mn', 'pl', 'pr', 'rh', 'tr', 'qu', 'wh', 'q'
];
$start = mt_rand(0, 2);
$start = random_int(0, 2);
if ($start == 0) {
$table = $vowels;
} else {
@ -127,8 +132,8 @@ class Strings
$word = '';
for ($x = 0; $x < $len; $x ++) {
$r = mt_rand(0, count($table) - 1);
for ($x = 0; $x < $len; $x++) {
$r = random_int(0, count($table) - 1);
$word .= $table[$r];
if ($table == $vowels) {
@ -136,7 +141,6 @@ class Strings
} else {
$table = $vowels;
}
}
$word = substr($word, 0, $len);
@ -152,20 +156,20 @@ class Strings
return $word;
}
/**
* Translate and format the network name of a contact
*
* @param string $network Network name of the contact (e.g. dfrn, rss and so on)
* @param string $url The contact url
*
* @return string Formatted network name
* @throws \Friendica\Network\HTTPException\InternalServerErrorException
*/
/**
* Translate and format the network name of a contact
*
* @param string $network Network name of the contact (e.g. dfrn, rss and so on)
* @param string $url The contact url
*
* @return string Formatted network name
* @throws \Friendica\Network\HTTPException\InternalServerErrorException
*/
public static function formatNetworkName($network, $url = '')
{
if ($network != '') {
if ($url != '') {
$network_name = '<a href="' . $url .'">' . ContactSelector::networkToName($network, $url) . '</a>';
$network_name = '<a href="' . $url . '">' . ContactSelector::networkToName($network, $url) . '</a>';
} 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 '@
(?<![=\'\]"/]) # Not preceded by [, =, \', ], ", /
\b
( # Capture 1: entire matched URL
https?:// # http or https protocol
(?:
[^/\s\xA0`!()\[\]{};:\'",<>?«»“”‘’.] # 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;
}
}