Skip to content

Commit 5438ae9

Browse files
fix: prevent token leak via URL userinfo host confusion (#2000)
enhance URL validation to prevent host-confusion attacks update URL parsing to reject userinfo and prevent host-confusion attacks
1 parent db1757a commit 5438ae9

5 files changed

Lines changed: 106 additions & 30 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@microsoft/microsoft-graph-client",
3-
"version": "3.0.7",
3+
"version": "3.0.8",
44
"description": "Microsoft Graph Client Library",
55
"keywords": [
66
"Microsoft",

src/GraphRequest.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,25 @@ export class GraphRequest {
132132
private parsePath = (path: string): void => {
133133
// Strips out the base of the url if they passed in
134134
if (path.indexOf("https://") !== -1) {
135-
path = path.replace("https://", "");
135+
// Use structured URL parsing to validate the URL and reject userinfo
136+
let parsedUrl: URL;
137+
try {
138+
parsedUrl = new URL(path);
139+
} catch {
140+
throw new GraphClientError("Unable to parse the URL: " + path);
141+
}
142+
// Reject URLs with userinfo to prevent host-confusion attacks
143+
if (parsedUrl.username !== "" || parsedUrl.password !== "") {
144+
throw new GraphClientError("URL cannot contain user credentials: " + path);
145+
}
136146

137-
// Find where the host ends
138-
const endOfHostStrPos = path.indexOf("/");
147+
// Extract host from the original string using the first "/" after "https://"
148+
const withoutScheme = path.substring("https://".length);
149+
const endOfHostStrPos = withoutScheme.indexOf("/");
139150
if (endOfHostStrPos !== -1) {
140-
// Parse out the host
141-
this.urlComponents.host = "https://" + path.substring(0, endOfHostStrPos);
151+
this.urlComponents.host = "https://" + withoutScheme.substring(0, endOfHostStrPos);
142152
// Strip the host from path
143-
path = path.substring(endOfHostStrPos + 1, path.length);
153+
path = withoutScheme.substring(endOfHostStrPos + 1, withoutScheme.length);
144154
}
145155

146156
// Remove the following version

src/GraphRequestUtil.ts

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -87,29 +87,20 @@ export const isCustomHost = (url: string, customHosts: Set<string>): boolean =>
8787
* @returns {boolean} - Returns true is for one of the provided endpoints.
8888
*/
8989
const isValidEndpoint = (url: string, allowedHosts: Set<string> = GRAPH_URLS): boolean => {
90-
// Valid Graph URL pattern - https://graph.microsoft.com/{version}/{resource}?{query-parameters}
91-
// Valid Graph URL example - https://graph.microsoft.com/v1.0/
92-
url = url.toLowerCase();
93-
94-
if (url.indexOf("https://") !== -1) {
95-
url = url.replace("https://", "");
96-
97-
// Find where the host ends
98-
const startofPortNoPos = url.indexOf(":");
99-
const endOfHostStrPos = url.indexOf("/");
100-
let hostName = "";
101-
if (endOfHostStrPos !== -1) {
102-
if (startofPortNoPos !== -1 && startofPortNoPos < endOfHostStrPos) {
103-
hostName = url.substring(0, startofPortNoPos);
104-
return allowedHosts.has(hostName);
105-
}
106-
// Parse out the host
107-
hostName = url.substring(0, endOfHostStrPos);
108-
return allowedHosts.has(hostName);
109-
}
90+
let parsedUrl: URL;
91+
try {
92+
parsedUrl = new URL(url);
93+
} catch {
94+
return false;
11095
}
111-
112-
return false;
96+
if (parsedUrl.protocol !== "https:") {
97+
return false;
98+
}
99+
// Reject URLs with userinfo to prevent host-confusion attacks
100+
if (parsedUrl.username !== "" || parsedUrl.password !== "") {
101+
return false;
102+
}
103+
return allowedHosts.has(parsedUrl.hostname.toLowerCase());
113104
};
114105

115106
/**

test/common/core/GraphRequestUtil.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { assert } from "chai";
99

10-
import { serializeContent, urlJoin } from "../../../src/GraphRequestUtil";
10+
import { serializeContent, urlJoin, isGraphURL, isCustomHost } from "../../../src/GraphRequestUtil";
1111

1212
describe("GraphRequestUtil.ts", () => {
1313
describe("urlJoin", () => {
@@ -77,4 +77,49 @@ describe("GraphRequestUtil.ts", () => {
7777
assert.equal(serializeContent(val), "null");
7878
});
7979
});
80+
81+
describe("isGraphURL - host confusion regression", () => {
82+
it("Should accept valid Graph URLs", () => {
83+
assert.isTrue(isGraphURL("https://graph.microsoft.com/v1.0/me"));
84+
assert.isTrue(isGraphURL("https://graph.microsoft.com:443/v1.0/me"));
85+
assert.isTrue(isGraphURL("https://graph.microsoft.us/v1.0/me"));
86+
assert.isTrue(isGraphURL("https://dod-graph.microsoft.us/v1.0/me"));
87+
assert.isTrue(isGraphURL("https://graph.microsoft.de/v1.0/me"));
88+
assert.isTrue(isGraphURL("https://microsoftgraph.chinacloudapi.cn/v1.0/me"));
89+
assert.isTrue(isGraphURL("https://canary.graph.microsoft.com/v1.0/me"));
90+
});
91+
92+
it("Should reject URLs with userinfo (host confusion attack)", () => {
93+
assert.isFalse(isGraphURL("https://graph.microsoft.com:443@attacker.example/v1.0/me"));
94+
assert.isFalse(isGraphURL("https://graph.microsoft.com:8080@attacker.example/v1.0/me"));
95+
assert.isFalse(isGraphURL("https://graph.microsoft.com@attacker.example/v1.0/me"));
96+
assert.isFalse(isGraphURL("https://user:pass@graph.microsoft.com/v1.0/me"));
97+
});
98+
99+
it("Should reject non-Graph hosts", () => {
100+
assert.isFalse(isGraphURL("https://attacker.example/v1.0/me"));
101+
assert.isFalse(isGraphURL("https://graph.microsoft.com.evil.example/v1.0/me"));
102+
});
103+
104+
it("Should reject non-HTTPS URLs", () => {
105+
assert.isFalse(isGraphURL("http://graph.microsoft.com/v1.0/me"));
106+
});
107+
108+
it("Should reject malformed URLs", () => {
109+
assert.isFalse(isGraphURL("not-a-url"));
110+
assert.isFalse(isGraphURL(""));
111+
});
112+
});
113+
114+
describe("isCustomHost - host confusion regression", () => {
115+
const customHosts = new Set<string>(["api.example.com"]);
116+
117+
it("Should accept valid custom host URLs", () => {
118+
assert.isTrue(isCustomHost("https://api.example.com/v1.0/data", customHosts));
119+
});
120+
121+
it("Should reject URLs with userinfo targeting custom hosts", () => {
122+
assert.isFalse(isCustomHost("https://api.example.com:443@attacker.example/v1.0/data", customHosts));
123+
});
124+
});
80125
});

test/common/core/urlParsing.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,34 @@ describe("urlParsing.ts", () => {
6161
}
6262
}
6363
});
64+
65+
describe("parsePath - userinfo rejection (security)", () => {
66+
it("should throw for URLs with userinfo (host confusion attack)", () => {
67+
assert.throws(() => {
68+
client.api("https://graph.microsoft.com:443@attacker.example/v1.0/me");
69+
}, /URL cannot contain user credentials/);
70+
});
71+
72+
it("should throw for URLs with username only", () => {
73+
assert.throws(() => {
74+
client.api("https://graph.microsoft.com@attacker.example/v1.0/me");
75+
}, /URL cannot contain user credentials/);
76+
});
77+
78+
it("should throw for URLs with user:pass", () => {
79+
assert.throws(() => {
80+
client.api("https://user:pass@graph.microsoft.com/v1.0/me");
81+
}, /URL cannot contain user credentials/);
82+
});
83+
84+
it("should accept valid absolute URLs without userinfo", () => {
85+
const request = client.api("https://graph.microsoft.com/v1.0/me");
86+
assert.equal(request["buildFullUrl"](), "https://graph.microsoft.com/v1.0/me");
87+
});
88+
89+
it("should accept valid absolute URLs with port", () => {
90+
const request = client.api("https://graph.microsoft.com:443/v1.0/me");
91+
assert.equal(request["buildFullUrl"](), "https://graph.microsoft.com:443/v1.0/me");
92+
});
93+
});
6494
});

0 commit comments

Comments
 (0)