Skip to content

Commit

Permalink
Fix untrusted redirect vulnrability.
Browse files Browse the repository at this point in the history
  • Loading branch information
EarthlingDavey committed Apr 10, 2024
1 parent 8d9c4a1 commit 6d111ea
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 19 deletions.
10 changes: 7 additions & 3 deletions public/app/mu-plugins/moj-auth/jwt.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function getJwt(): bool | object
return false;
}

if($decoded && $decoded->sub) {
if ($decoded && $decoded->sub) {
$this->sub = $decoded->sub;
}

Expand All @@ -73,18 +73,22 @@ public function getJwt(): bool | object
* @return object Returns the JWT payload.
*/

public function setJwt(array $args = []) : object
public function setJwt(array $args = []): object
{
$this->log('setJwt()');

$expiry = isset($args['expiry']) ? $args['expiry'] : $this->now + $this::JWT_DURATION;

if (!$this->sub) {
$this->sub = bin2hex(random_bytes(16));
}

$payload = [
// Registered claims - https://datatracker.ietf.org/doc/html/rfc7519#section-4.1
'sub' => $this->sub,
'exp' => $expiry,
// Public claims - https://www.iana.org/assignments/jwt/jwt.xhtml
'roles' => ['reader']
'roles' => isset($args['roles']) ? $args['roles'] : [],
];

$jwt = JWT::encode($payload, $this->jwt_secret, $this::JWT_ALGORITHM);
Expand Down
22 changes: 12 additions & 10 deletions public/app/mu-plugins/moj-auth/moj-auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ public function handlePageRequest(string $required_role = 'reader'): void
// Get the JWT token from the request. Do this early so that we populate $this->sub if it's known.
$jwt = $this->getJwt();

// If there was no JWT and subsequently no user ID, let's create one and set it.
if(empty($this->sub)) {
$this->sub = bin2hex(random_bytes(16));
// Set a JWT without a role, to persist the user's ID.
if (!$jwt) {
$jwt = $this->setJwt();
}

// If we've hit the callback endpoint, then handle it here. On fail it exits with 401 & php code execution stops here.
Expand All @@ -91,15 +91,16 @@ public function handlePageRequest(string $required_role = 'reader'): void
// Set a JWT cookie.
$this->setJwt([
'expiry' => $oauth_access_token->getExpires(),
'roles' => ['reader']
]);
// Store the tokens.
$this->storeTokens($this->sub, $oauth_access_token, 'refresh');
// Get the origin request from the cookie.
$user_redirect = \home_url($_COOKIE[$this::OAUTH_USER_URL_COOKIE_NAME] ?? '/');
// Remove the cookie.
$this->deleteCookie($this::OAUTH_USER_URL_COOKIE_NAME);
$user_redirect = get_transient('oauth_user_url_' . $this->sub);
// Remove the transient.
delete_transient('oauth_user_url_' . $this->sub);
// Redirect the user to the page they were trying to access.
header('Location: ' . $user_redirect);
header('Location: ' . \home_url($user_redirect ?? '/'));
exit();
}

Expand All @@ -120,7 +121,7 @@ public function handlePageRequest(string $required_role = 'reader'): void

// If the IP address is allowed, set a JWT and return.
if ($this->ipAddressIsAllowed()) {
$this->setJwt();
$this->setJwt(['roles' => ['reader']]);
return;
}

Expand All @@ -133,18 +134,19 @@ public function handlePageRequest(string $required_role = 'reader'): void
// Set a JWT cookie.
$jwt = $this->setJwt([
'expiry' => $oauth_access_token->getExpires(),
'roles' => ['reader']
]);
// Store the tokens.
$this->storeTokens($this->sub, $oauth_refreshed_access_token, 'refresh');
return;
}

// If there's any time left on the JWT then return.
if ($jwt_remaining_time > 0) {
if ($jwt_correct_role && $jwt_remaining_time > 0) {
return;
}

// Handle Azure AD/Entra ID OAuth. It redirects to Azure or xeits with 401 if disabled. php code execution always stops here.
// Handle Azure AD/Entra ID OAuth. It redirects to Azure or exits with 401 if disabled. php code execution always stops here.
$this->oauthLogin();
}

Expand Down
13 changes: 7 additions & 6 deletions public/app/mu-plugins/moj-auth/oauth.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ public function initOauth()
$this->log('initOauth()');

// Check for required environment variables. OAuth can be disable by not setting these.
if(empty($_ENV['OAUTH_TENNANT_ID']) || empty($_ENV['OAUTH_CLIENT_ID']) || empty($_ENV['OAUTH_CLIENT_SECRET'])) {
$this->oauth_enabled = !empty($_ENV['OAUTH_TENNANT_ID']) && !empty($_ENV['OAUTH_CLIENT_ID']) && !empty($_ENV['OAUTH_CLIENT_SECRET']);

if (!$this->oauth_enabled) {
$this->log('Missing OAuth environment variables');
$this->oauth_enabled = false;
return;
}

Expand Down Expand Up @@ -94,7 +95,7 @@ public function oauthLogin(): void
{
$this->log('oauthLogin()');

if(!$this->oauth_enabled) {
if (!$this->oauth_enabled) {
$this->log('OAuth is not enabled');
http_response_code(401) && exit();
}
Expand All @@ -109,8 +110,8 @@ public function oauthLogin(): void
// Use a cookie to store oauth state.
$this->setCookie($this::OAUTH_STATE_COOKIE_NAME, $state_hashed, -1);

// Store the user's origin URL in a cookie.
$this->setCookie($this::OAUTH_USER_URL_COOKIE_NAME, $_SERVER['REQUEST_URI'] ?? '', -1);
// Store the user's origin URL in a transient.
set_transient('oauth_user_url_' . $this->sub, $_SERVER['REQUEST_URI'] ?? '', 60 * 5); // 5 minutes

// Storing pkce prevents an attacker from potentially intercepting the auth code and using it.
set_transient('oauth_pkce_' . $state_hashed, $oauth_client->getPkceCode(), 60 * 5); // 5 minutes
Expand All @@ -132,7 +133,7 @@ public function oauthCallback(): AccessTokenInterface
{
$this->log('oauthCallback()');

if(!$this->oauth_enabled) {
if (!$this->oauth_enabled) {
$this->log('OAuth is not enabled');
http_response_code(401) && exit();
}
Expand Down
7 changes: 7 additions & 0 deletions public/app/mu-plugins/moj-auth/utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ public function ipAddressIsAllowed(): bool
return $this->ipMatch($_SERVER['REMOTE_ADDR'], $allowedIps);
}

/**
* Hash a value using SHA256 and a salt.
*
* @param string $value The value to hash.
* @return string The hashed value.
*/

public function hash(string $value): string
{
$this->log('hash()');
Expand Down

0 comments on commit 6d111ea

Please sign in to comment.