diff --git a/package.json b/package.json index 211595d57..ef5aeef45 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/microsoft-graph-client", - "version": "3.0.7", + "version": "3.0.8", "description": "Microsoft Graph Client Library", "keywords": [ "Microsoft", diff --git a/src/GraphRequest.ts b/src/GraphRequest.ts index 80a9e2c8c..cfa91d33e 100644 --- a/src/GraphRequest.ts +++ b/src/GraphRequest.ts @@ -132,15 +132,25 @@ export class GraphRequest { private parsePath = (path: string): void => { // Strips out the base of the url if they passed in if (path.indexOf("https://") !== -1) { - path = path.replace("https://", ""); + // Use structured URL parsing to validate the URL and reject userinfo + let parsedUrl: URL; + try { + parsedUrl = new URL(path); + } catch { + throw new GraphClientError("Unable to parse the URL: " + path); + } + // Reject URLs with userinfo to prevent host-confusion attacks + if (parsedUrl.username !== "" || parsedUrl.password !== "") { + throw new GraphClientError("URL cannot contain user credentials: " + path); + } - // Find where the host ends - const endOfHostStrPos = path.indexOf("/"); + // Extract host from the original string using the first "/" after "https://" + const withoutScheme = path.substring("https://".length); + const endOfHostStrPos = withoutScheme.indexOf("/"); if (endOfHostStrPos !== -1) { - // Parse out the host - this.urlComponents.host = "https://" + path.substring(0, endOfHostStrPos); + this.urlComponents.host = "https://" + withoutScheme.substring(0, endOfHostStrPos); // Strip the host from path - path = path.substring(endOfHostStrPos + 1, path.length); + path = withoutScheme.substring(endOfHostStrPos + 1, withoutScheme.length); } // Remove the following version diff --git a/src/GraphRequestUtil.ts b/src/GraphRequestUtil.ts index 02cf6c156..03930dfa7 100644 --- a/src/GraphRequestUtil.ts +++ b/src/GraphRequestUtil.ts @@ -87,29 +87,20 @@ export const isCustomHost = (url: string, customHosts: Set): boolean => * @returns {boolean} - Returns true is for one of the provided endpoints. */ const isValidEndpoint = (url: string, allowedHosts: Set = GRAPH_URLS): boolean => { - // Valid Graph URL pattern - https://graph.microsoft.com/{version}/{resource}?{query-parameters} - // Valid Graph URL example - https://graph.microsoft.com/v1.0/ - url = url.toLowerCase(); - - if (url.indexOf("https://") !== -1) { - url = url.replace("https://", ""); - - // Find where the host ends - const startofPortNoPos = url.indexOf(":"); - const endOfHostStrPos = url.indexOf("/"); - let hostName = ""; - if (endOfHostStrPos !== -1) { - if (startofPortNoPos !== -1 && startofPortNoPos < endOfHostStrPos) { - hostName = url.substring(0, startofPortNoPos); - return allowedHosts.has(hostName); - } - // Parse out the host - hostName = url.substring(0, endOfHostStrPos); - return allowedHosts.has(hostName); - } + let parsedUrl: URL; + try { + parsedUrl = new URL(url); + } catch { + return false; } - - return false; + if (parsedUrl.protocol !== "https:") { + return false; + } + // Reject URLs with userinfo to prevent host-confusion attacks + if (parsedUrl.username !== "" || parsedUrl.password !== "") { + return false; + } + return allowedHosts.has(parsedUrl.hostname.toLowerCase()); }; /** diff --git a/test/common/core/GraphRequestUtil.ts b/test/common/core/GraphRequestUtil.ts index 30704ac9b..09438784b 100644 --- a/test/common/core/GraphRequestUtil.ts +++ b/test/common/core/GraphRequestUtil.ts @@ -7,7 +7,7 @@ import { assert } from "chai"; -import { serializeContent, urlJoin } from "../../../src/GraphRequestUtil"; +import { serializeContent, urlJoin, isGraphURL, isCustomHost } from "../../../src/GraphRequestUtil"; describe("GraphRequestUtil.ts", () => { describe("urlJoin", () => { @@ -77,4 +77,49 @@ describe("GraphRequestUtil.ts", () => { assert.equal(serializeContent(val), "null"); }); }); + + describe("isGraphURL - host confusion regression", () => { + it("Should accept valid Graph URLs", () => { + assert.isTrue(isGraphURL("https://graph.microsoft.com/v1.0/me")); + assert.isTrue(isGraphURL("https://graph.microsoft.com:443/v1.0/me")); + assert.isTrue(isGraphURL("https://graph.microsoft.us/v1.0/me")); + assert.isTrue(isGraphURL("https://dod-graph.microsoft.us/v1.0/me")); + assert.isTrue(isGraphURL("https://graph.microsoft.de/v1.0/me")); + assert.isTrue(isGraphURL("https://microsoftgraph.chinacloudapi.cn/v1.0/me")); + assert.isTrue(isGraphURL("https://canary.graph.microsoft.com/v1.0/me")); + }); + + it("Should reject URLs with userinfo (host confusion attack)", () => { + assert.isFalse(isGraphURL("https://graph.microsoft.com:443@attacker.example/v1.0/me")); + assert.isFalse(isGraphURL("https://graph.microsoft.com:8080@attacker.example/v1.0/me")); + assert.isFalse(isGraphURL("https://graph.microsoft.com@attacker.example/v1.0/me")); + assert.isFalse(isGraphURL("https://user:pass@graph.microsoft.com/v1.0/me")); + }); + + it("Should reject non-Graph hosts", () => { + assert.isFalse(isGraphURL("https://attacker.example/v1.0/me")); + assert.isFalse(isGraphURL("https://graph.microsoft.com.evil.example/v1.0/me")); + }); + + it("Should reject non-HTTPS URLs", () => { + assert.isFalse(isGraphURL("http://graph.microsoft.com/v1.0/me")); + }); + + it("Should reject malformed URLs", () => { + assert.isFalse(isGraphURL("not-a-url")); + assert.isFalse(isGraphURL("")); + }); + }); + + describe("isCustomHost - host confusion regression", () => { + const customHosts = new Set(["api.example.com"]); + + it("Should accept valid custom host URLs", () => { + assert.isTrue(isCustomHost("https://api.example.com/v1.0/data", customHosts)); + }); + + it("Should reject URLs with userinfo targeting custom hosts", () => { + assert.isFalse(isCustomHost("https://api.example.com:443@attacker.example/v1.0/data", customHosts)); + }); + }); }); diff --git a/test/common/core/urlParsing.ts b/test/common/core/urlParsing.ts index 4f83a02ea..a50a544d3 100644 --- a/test/common/core/urlParsing.ts +++ b/test/common/core/urlParsing.ts @@ -61,4 +61,34 @@ describe("urlParsing.ts", () => { } } }); + + describe("parsePath - userinfo rejection (security)", () => { + it("should throw for URLs with userinfo (host confusion attack)", () => { + assert.throws(() => { + client.api("https://graph.microsoft.com:443@attacker.example/v1.0/me"); + }, /URL cannot contain user credentials/); + }); + + it("should throw for URLs with username only", () => { + assert.throws(() => { + client.api("https://graph.microsoft.com@attacker.example/v1.0/me"); + }, /URL cannot contain user credentials/); + }); + + it("should throw for URLs with user:pass", () => { + assert.throws(() => { + client.api("https://user:pass@graph.microsoft.com/v1.0/me"); + }, /URL cannot contain user credentials/); + }); + + it("should accept valid absolute URLs without userinfo", () => { + const request = client.api("https://graph.microsoft.com/v1.0/me"); + assert.equal(request["buildFullUrl"](), "https://graph.microsoft.com/v1.0/me"); + }); + + it("should accept valid absolute URLs with port", () => { + const request = client.api("https://graph.microsoft.com:443/v1.0/me"); + assert.equal(request["buildFullUrl"](), "https://graph.microsoft.com:443/v1.0/me"); + }); + }); });