mirror of
https://github.com/bitwarden/browser
synced 2025-12-12 22:33:35 +00:00
[PM-18888] Fix duo redirect URL checks (#14174)
* fix(PM-18888) : Create more strict checking of redirectURL to protect against open redirect attacks using regex. * fix : modify comments and check for embedded credentials. * feat : add testability to duo-redirect connector * fix : fixing strict typing; Removed styling from duo-redirect.ts which allows us to test without adding additional files and configurations for jest. * fix : remove duo-redirect.scss
This commit is contained in:
@@ -1 +0,0 @@
|
||||
@import "../scss/styles.scss";
|
||||
51
apps/web/src/connectors/duo-redirect.spec.ts
Normal file
51
apps/web/src/connectors/duo-redirect.spec.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import { redirectToDuoFrameless } from "./duo-redirect";
|
||||
|
||||
describe("duo-redirect", () => {
|
||||
describe("redirectToDuoFrameless", () => {
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(window, "location", {
|
||||
value: { href: "" },
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("should redirect to a valid Duo URL", () => {
|
||||
const validUrl = "https://api-123.duosecurity.com/auth";
|
||||
redirectToDuoFrameless(validUrl);
|
||||
expect(window.location.href).toBe(validUrl);
|
||||
});
|
||||
|
||||
it("should redirect to a valid Duo Federal URL", () => {
|
||||
const validUrl = "https://api-123.duofederal.com/auth";
|
||||
redirectToDuoFrameless(validUrl);
|
||||
expect(window.location.href).toBe(validUrl);
|
||||
});
|
||||
|
||||
it("should throw an error for an invalid URL", () => {
|
||||
const invalidUrl = "https://malicious-site.com";
|
||||
expect(() => redirectToDuoFrameless(invalidUrl)).toThrow("Invalid redirect URL");
|
||||
});
|
||||
|
||||
it("should throw an error for an malicious URL with valid redirect embedded", () => {
|
||||
const invalidUrl = "https://malicious-site.com\\@api-123.duosecurity.com/auth";
|
||||
expect(() => redirectToDuoFrameless(invalidUrl)).toThrow("Invalid redirect URL");
|
||||
});
|
||||
|
||||
it("should throw an error for a non-HTTPS URL", () => {
|
||||
const nonHttpsUrl = "http://api-123.duosecurity.com/auth";
|
||||
expect(() => redirectToDuoFrameless(nonHttpsUrl)).toThrow("Invalid redirect URL");
|
||||
});
|
||||
|
||||
it("should throw an error for a URL with an invalid hostname", () => {
|
||||
const invalidHostnameUrl = "https://api-123.invalid.com";
|
||||
expect(() => redirectToDuoFrameless(invalidHostnameUrl)).toThrow("Invalid redirect URL");
|
||||
});
|
||||
|
||||
it("should throw an error for a URL with credentials", () => {
|
||||
const UrlWithCredentials = "https://api-123.duosecurity.com:password@evil/attack";
|
||||
expect(() => redirectToDuoFrameless(UrlWithCredentials)).toThrow(
|
||||
"Invalid redirect URL: embedded credentials not allowed",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,14 +1,8 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
import { getQsParam } from "./common";
|
||||
import { TranslationService } from "./translation.service";
|
||||
|
||||
// FIXME: Remove when updating file. Eslint update
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
require("./duo-redirect.scss");
|
||||
|
||||
const mobileDesktopCallback = "bitwarden://duo-callback";
|
||||
let localeService: TranslationService = null;
|
||||
let localeService: TranslationService | null = null;
|
||||
|
||||
window.addEventListener("load", async () => {
|
||||
const redirectUrl = getQsParam("duoFramelessUrl");
|
||||
@@ -18,9 +12,18 @@ window.addEventListener("load", async () => {
|
||||
return;
|
||||
}
|
||||
|
||||
const client = getQsParam("client");
|
||||
const code = getQsParam("code");
|
||||
const state = getQsParam("state");
|
||||
const client: string | null = getQsParam("client");
|
||||
const code: string | null = getQsParam("code");
|
||||
const state: string | null = getQsParam("state");
|
||||
if (!client) {
|
||||
throw new Error("client is null");
|
||||
}
|
||||
if (!code) {
|
||||
throw new Error("code is null");
|
||||
}
|
||||
if (!state) {
|
||||
throw new Error("state is null");
|
||||
}
|
||||
|
||||
localeService = new TranslationService(navigator.language, "locales");
|
||||
await localeService.init();
|
||||
@@ -53,16 +56,28 @@ window.addEventListener("load", async () => {
|
||||
* validate the Duo AuthUrl and redirect to it.
|
||||
* @param redirectUrl the duo auth url
|
||||
*/
|
||||
function redirectToDuoFrameless(redirectUrl: string) {
|
||||
const validateUrl = new URL(redirectUrl);
|
||||
const validDuoUrl =
|
||||
validateUrl.protocol === "https:" &&
|
||||
(validateUrl.hostname.endsWith(".duosecurity.com") ||
|
||||
validateUrl.hostname.endsWith(".duofederal.com"));
|
||||
|
||||
if (!validDuoUrl) {
|
||||
export function redirectToDuoFrameless(redirectUrl: string) {
|
||||
// Regex to match a valid duo redirect URL
|
||||
/**
|
||||
* This regex checks for the following:
|
||||
* The string must start with "https://api-"
|
||||
* Followed by a subdomain that can contain letters, numbers
|
||||
* Followed by either "duosecurity.com" or "duofederal.com"
|
||||
* This ensures that the redirect does not contain any malicious content
|
||||
* and is a valid Duo URL.
|
||||
* */
|
||||
const duoRedirectUrlRegex = /^https:\/\/api-[a-zA-Z0-9]+\.(duosecurity|duofederal)\.com/;
|
||||
// Check if the redirect URL matches the regex
|
||||
if (!duoRedirectUrlRegex.test(redirectUrl)) {
|
||||
throw new Error("Invalid redirect URL");
|
||||
}
|
||||
// At this point we know the URL to be valid, but we need to check for embedded credentials
|
||||
const validateUrl = new URL(redirectUrl);
|
||||
// URLs should not contain
|
||||
// Check that no embedded credentials are present
|
||||
if (validateUrl.username || validateUrl.password) {
|
||||
throw new Error("Invalid redirect URL: embedded credentials not allowed");
|
||||
}
|
||||
|
||||
window.location.href = decodeURIComponent(redirectUrl);
|
||||
}
|
||||
@@ -72,17 +87,23 @@ function redirectToDuoFrameless(redirectUrl: string) {
|
||||
* so browser, desktop, and mobile are not able to take advantage of the countdown timer or close button.
|
||||
*/
|
||||
function displayHandoffMessage(client: string) {
|
||||
const content = document.getElementById("content");
|
||||
const content: HTMLElement | null = document.getElementById("content");
|
||||
if (!content) {
|
||||
throw new Error("content element not found");
|
||||
}
|
||||
content.className = "text-center";
|
||||
content.innerHTML = "";
|
||||
|
||||
const h1 = document.createElement("h1");
|
||||
const p = document.createElement("p");
|
||||
const p: HTMLElement = document.createElement("p");
|
||||
|
||||
if (!localeService) {
|
||||
throw new Error("localeService is not initialized");
|
||||
}
|
||||
h1.textContent = localeService.t("youSuccessfullyLoggedIn");
|
||||
p.textContent =
|
||||
client == "web"
|
||||
? (p.textContent = localeService.t("thisWindowWillCloseIn5Seconds"))
|
||||
? localeService.t("thisWindowWillCloseIn5Seconds")
|
||||
: localeService.t("youMayCloseThisWindow");
|
||||
|
||||
h1.className = "font-weight-semibold";
|
||||
@@ -102,11 +123,20 @@ function displayHandoffMessage(client: string) {
|
||||
});
|
||||
content.appendChild(button);
|
||||
|
||||
// Countdown timer (closes tab upon completion)
|
||||
let num = Number(p.textContent.match(/\d+/)[0]);
|
||||
if (p.textContent === null) {
|
||||
throw new Error("count down container is null");
|
||||
}
|
||||
const counterString: string | null = p.textContent.match(/\d+/)?.[0] || null;
|
||||
if (!counterString) {
|
||||
throw new Error("count down time cannot be null");
|
||||
}
|
||||
|
||||
let num: number = Number(counterString);
|
||||
const interval = setInterval(() => {
|
||||
if (num > 1) {
|
||||
if (p.textContent === null) {
|
||||
throw new Error("count down container is null");
|
||||
}
|
||||
p.textContent = p.textContent.replace(String(num), String(num - 1));
|
||||
num--;
|
||||
} else {
|
||||
|
||||
@@ -142,7 +142,7 @@ const plugins = [
|
||||
new HtmlWebpackPlugin({
|
||||
template: "./src/connectors/duo-redirect.html",
|
||||
filename: "duo-redirect-connector.html",
|
||||
chunks: ["connectors/duo-redirect"],
|
||||
chunks: ["connectors/duo-redirect", "styles"],
|
||||
}),
|
||||
new HtmlWebpackPlugin({
|
||||
template: "./src/404.html",
|
||||
|
||||
Reference in New Issue
Block a user