Skip to content

Commit

Permalink
returnUrl open-redirect security bug fixed (#1975)
Browse files Browse the repository at this point in the history
  • Loading branch information
JMach1 committed Oct 20, 2022
1 parent c5b68e7 commit fb62ef0
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 18 deletions.
9 changes: 4 additions & 5 deletions src/components/defaultAuthenticator.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as Constants from "./../constants";
import { sanitizeUrl } from "@braintree/sanitize-url";
import { AccessToken, IAuthenticator } from "./../authentication";

import { AccessToken, IAuthenticator } from "../authentication";
import { Utils } from "../utils";

export class DefaultAuthenticator implements IAuthenticator {
private runSsoFlow(): Promise<void> {
Expand All @@ -19,8 +18,8 @@ export class DefaultAuthenticator implements IAuthenticator {

await this.setAccessToken(token);

// wait for redirect to happen, deliberatly not resolving the promise
window.location.assign(sanitizeUrl(returnUrl));
// wait for redirect to happen, deliberately not resolving the promise
window.location.assign(Utils.sanitizeReturnUrl(returnUrl));
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/users/signin/ko/runtime/signin.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as ko from "knockout";
import * as validation from "knockout.validation";
import template from "./signin.html";
import { sanitizeUrl } from "@braintree/sanitize-url";
import { EventManager } from "@paperbits/common/events";
import { Component, OnMounted, Param, RuntimeComponent } from "@paperbits/common/ko/decorators";
import { Router } from "@paperbits/common/routing/router";
Expand All @@ -12,6 +11,7 @@ import { UsersService } from "../../../../../services";
import { dispatchErrors } from "../../../validation-summary/utils";
import { ErrorSources } from "../../../validation-summary/constants";
import { ValidationMessages } from "../../../validationMessages";
import { Utils } from "../../../../../utils";

@RuntimeComponent({
selector: "signin-runtime"
Expand Down Expand Up @@ -40,7 +40,7 @@ export class Signin {
this.password = ko.observable("");
this.errorMessages = ko.observableArray([]);
this.hasErrors = ko.pureComputed(() => this.errorMessages().length > 0);
// Next four variables are necessary for displaying Terms of Use. Will be called when the back-end implementation is done
// Next four variables are necessary for displaying Terms of Use. Will be called when the back-end implementation is done
this.termsEnabled = ko.observable(false);
this.termsOfUse = ko.observable();
this.isConsentRequired = ko.observable(false);
Expand Down Expand Up @@ -127,7 +127,7 @@ export class Signin {
const returnUrl = this.routeHelper.getQueryParameter("returnUrl") || clientReturnUrl;

if (returnUrl) {
await this.router.navigateTo(sanitizeUrl(returnUrl));
await this.router.navigateTo(Utils.sanitizeReturnUrl(returnUrl));
return;
}

Expand Down
3 changes: 1 addition & 2 deletions src/services/aadService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as Msal from "msal";
import * as Constants from "../constants";
import { sanitizeUrl } from "@braintree/sanitize-url";
import { HttpClient } from "@paperbits/common/http";
import { Router } from "@paperbits/common/routing";
import { RouteHelper } from "../routing/routeHelper";
Expand Down Expand Up @@ -53,7 +52,7 @@ export class AadService implements IAadService {

this.router.getCurrentUrl() === returnUrl
? location.reload()
: await this.router.navigateTo(sanitizeUrl(returnUrl));
: await this.router.navigateTo(Utils.sanitizeReturnUrl(returnUrl));
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/services/aadServiceV2.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as msal from "@azure/msal-browser";
import * as Constants from "../constants";
import { sanitizeUrl } from "@braintree/sanitize-url";
import { HttpClient } from "@paperbits/common/http";
import { Router } from "@paperbits/common/routing";
import { RouteHelper } from "../routing/routeHelper";
Expand Down Expand Up @@ -53,7 +52,7 @@ export class AadServiceV2 implements IAadService {

this.router.getCurrentUrl() === returnUrl
? location.reload()
: await this.router.navigateTo(sanitizeUrl(returnUrl));
: await this.router.navigateTo(Utils.sanitizeReturnUrl(returnUrl));
}

/**
Expand Down
24 changes: 18 additions & 6 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sanitizeUrl } from "@braintree/sanitize-url";
import { ArmResource } from "./contracts/armResource";
import { JwtToken } from "./contracts/jwtToken";
import { js } from "js-beautify";
Expand Down Expand Up @@ -81,6 +82,17 @@ export class Utils {
return result;
}

/**
* Returns relative part of the URL.
*/
public static getRelativeUrl(url: string): string {
return url.replace(/^(.*\/\/)?[^\/]*/gm, "")
}

public static sanitizeReturnUrl(returnUrl: string): string {
return sanitizeUrl(this.getRelativeUrl(returnUrl));
}

public static formatXml(xml: string): string {
const original = xml;

Expand Down Expand Up @@ -172,7 +184,7 @@ export class Utils {

/**
* Formats number of bytes into string.
* @param bytes
* @param bytes
*/
public static formatBytes(bytes: number): string {
let suffix = " bytes";
Expand Down Expand Up @@ -202,7 +214,7 @@ export class Utils {

/**
* Formats time span into string.
* @param bytes
* @param bytes
*/
public static formatTimespan(milliseconds: number): string {
let suffix = " ms";
Expand Down Expand Up @@ -270,7 +282,7 @@ export class Utils {
/**
* Encodes reserved URI character not encoded by the native encodeURI function
* (encodeURIComponent encodes many characters, which are desired unencoded)
*
*
* @param uri string to be encoded
* @param additionalReservedTuples optional array of additional reserved tuples to replace
* @returns encoded string
Expand Down Expand Up @@ -358,8 +370,8 @@ export class Utils {
}

/**
* This is a function used to generate long date format like (Weekday, Month, Day, Year)
*
* This is a function used to generate long date format like (Weekday, Month, Day, Year)
*
* @param time time string
*/
public static formatDateTime(time: string): string {
Expand Down Expand Up @@ -404,7 +416,7 @@ export class Utils {
public static isJsonContentType(contentType: string): boolean {
return /\bjson\b/i.test(contentType.toLocaleLowerCase());
}

public static isXmlContentType(contentType: string): boolean {
return /\bxml\b/i.test(contentType.toLocaleLowerCase());
}
Expand Down

0 comments on commit fb62ef0

Please sign in to comment.