Skip to content

Commit

Permalink
Security improvements
Browse files Browse the repository at this point in the history
Validity check of the timezone acquired by the user
  • Loading branch information
viames committed Aug 29, 2023
1 parent 4762017 commit fca033e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 41 deletions.
44 changes: 20 additions & 24 deletions src/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -609,11 +609,11 @@ private function getAcl(): ?array {
if (!$this->issetCache('acl')) {

$query =
'SELECT r.*, m.name AS module_name' .
' FROM `rules` AS r' .
' INNER JOIN `acl` AS a ON a.rule_id = r.id'.
' INNER JOIN `modules` AS m ON r.module_id = m.id'.
' WHERE a.group_id = ?';
'SELECT r.*, m.`name` AS `module_name`
FROM `rules` AS r
INNER JOIN `acl` AS a ON a.`rule_id` = r.`id`
INNER JOIN `modules` AS m ON r.`module_id` = m.`id`
WHERE a.`group_id` = ?';

$this->setCache('acl', Rule::getObjectsByQuery($query, [$this->groupId]));

Expand All @@ -631,12 +631,12 @@ private function getAcl(): ?array {
public function getLanding(): ?\stdClass {

$query =
' SELECT m.`name` AS module, r.action' .
' FROM `acl` AS a' .
' INNER JOIN `rules` AS r ON r.id = a.rule_id' .
' INNER JOIN `modules` AS m ON m.id = r.module_id' .
' WHERE a.is_default = 1' .
' AND a.group_id = ?';
'SELECT m.`name` AS `module`, r.`action`
FROM `acl` AS a
INNER JOIN `rules` AS r ON r.id = a.`rule_id`
INNER JOIN `modules` AS m ON m.`id` = r.`module_id`
WHERE a.`is_default` = 1
AND a.`group_id` = ?';

return Database::load($query, [$this->groupId], PAIR_DB_OBJECT);

Expand Down Expand Up @@ -664,11 +664,11 @@ public function getLanguageCode(): ?string {
if (!$this->issetCache('lang')) {

$query =
'SELECT l.code' .
' FROM `languages` AS l' .
' INNER JOIN `locales` AS lc ON l.id = lc.language_id' .
' INNER JOIN `users` AS u ON u.locale_id = lc.id' .
' WHERE u.id = ?';
'SELECT l.`code`
FROM `languages` AS l
INNER JOIN `locales` AS lc ON l.`id` = lc.`language_id`
INNER JOIN `users` AS u ON u.`locale_id` = lc.`id`
WHERE u.`id` = ?';

$this->setCache('lang', Database::load($query, [$this->id], PAIR_DB_RESULT));

Expand Down Expand Up @@ -756,11 +756,7 @@ public function isDeletable(): bool {
*/
public static function getByPwReset(string $pwReset): ?User {

$query =
'SELECT *' .
' FROM `users`' .
' WHERE `pw_reset` IS NOT NULL' .
' AND `pw_reset` = ?';
$query = 'SELECT * FROM `users` WHERE `pw_reset` IS NOT NULL AND `pw_reset` = ?';

return static::getObjectByQuery($query, [$pwReset]);

Expand Down Expand Up @@ -917,9 +913,9 @@ public function isAdmin(): bool {
return TRUE;
} else {
$query =
'SELECT COUNT(1) FROM `users`' .
' INNER JOIN `sessions` AS s ON u.id = s.id_user' .
' WHERE s.id = ? AND u.id = ? AND admin = 1';
'SELECT COUNT(1) FROM `users`
INNER JOIN `sessions` AS s ON u.`id` = s.`id_user`
WHERE s.`id` = ? AND u.`id` = ? AND `admin` = 1';
return (bool)Database::load($query, [Session::current(), $this->id], PAIR_DB_COUNT);
}

Expand Down
37 changes: 20 additions & 17 deletions src/UserRemember.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Pair;

class UserRemember extends ActiveRecord {

/**
* This property maps “user_id” column.
* @var int
Expand All @@ -15,10 +15,10 @@ class UserRemember extends ActiveRecord {
* @var string
*/
protected $rememberMe;

/**
* This property maps “created_at” column.
* @var DateTime
* @var DateTime|NULL
*/
protected $createdAt;

Expand All @@ -27,7 +27,7 @@ class UserRemember extends ActiveRecord {
* @var string
*/
const TABLE_NAME = 'users_remembers';

/**
* Name of primary key db field.
* @var array
Expand Down Expand Up @@ -61,16 +61,16 @@ public static function getUserByRememberMe(string $rememberMe): ?User {
' FROM `users` AS u' .
' INNER JOIN `users_remembers` AS ur ON u.`id` = ur.`user_id`' .
' WHERE ur.`remember_me` = ?';

$userClass = PAIR_USER_CLASS;

return $userClass::getObjectByQuery($query, [$rememberMe]);

}

/**
* Utility to unserialize and return the remember-me cookie content {timezone, rememberMe}.
*
*
* @return \stdClass|NULL
*/
public static function getCookieContent(): ?\stdClass {
Expand All @@ -82,36 +82,39 @@ public static function getCookieContent(): ?\stdClass {
if (!isset($_COOKIE[$cookieName])) {
return NULL;
}

// try to unserialize the cookie content
$content = unserialize($_COOKIE[$cookieName]);

// cookie content is not unserializable
if (FALSE === $content) {
return NULL;
}

$regex = "#^[123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVXYWZ]{32}$#";

// check if content exists and RememberMe length
if (isset($content[0]) and isset($content[1]) and 32==strlen($content[1])) {
if (is_array($content) and isset($content[0]) and isset($content[1]) and preg_match($regex, (string)$content[1])) {
$obj = new \stdClass();
$obj->timezone = $content[0];
$obj->rememberMe = $content[1];
$obj->timezone = ($content[0] and in_array($content[0], \DateTimeZone::listIdentifiers()))
? $content[0] : 'UTC';
$obj->rememberMe = (string)$content[1];
return $obj;
}

return NULL;

}

/**
* Build and return the cookie name.
*
*
* @return string
*/
public static function getCookieName(): string {

return Application::getCookiePrefix() . 'RememberMe';

}

}

0 comments on commit fca033e

Please sign in to comment.