Skip to content

Commit 707597d

Browse files
fix: add a maximum length for the URL
The regular expression used to parse the URL provided by the user has a time complexity of O(n^2), hence the length limitation. Please note that this does not seem realistically exploitable, as an attacker would have to be able to provide a malicious URL to the user and inject it in the Engine.IO client. We could also have: - modified the regex, but there are a lot of edge cases and the current test coverage is probably not sufficient - use the built-in URL object, but we would have to add a polyfill for old platforms like IE Thanks to Young-jin Hwang from the Soonchunhyang University for the responsible disclosure.
1 parent 8d86e0d commit 707597d

File tree

2 files changed

+8
-1
lines changed

2 files changed

+8
-1
lines changed

lib/contrib/parseuri.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ const parts = [
2323
'source', 'protocol', 'authority', 'userInfo', 'user', 'password', 'host', 'port', 'relative', 'path', 'directory', 'file', 'query', 'anchor'
2424
];
2525

26-
export function parse(str) {
26+
export function parse(str: string) {
27+
if (str.length > 2000) {
28+
throw "URI too long";
29+
}
30+
2731
const src = str,
2832
b = str.indexOf('['),
2933
e = str.indexOf(']');

test/parseuri.js

+3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// imported from https://github.com/galkn/parseuri
22
const expect = require("expect.js");
33
const parseuri = require("..").parse;
4+
const { repeat } = require("./util");
45

56
describe("parseuri", function () {
67
it("should parse an uri", function () {
@@ -64,5 +65,7 @@ describe("parseuri", function () {
6465
expect(relativeWithQuery.host).to.be("");
6566
expect(relativeWithQuery.path).to.be("/foo");
6667
expect(relativeWithQuery.query).to.be("bar=@example.com");
68+
69+
expect(() => parseuri(repeat("a", 2001))).to.throwError("URI too long");
6770
});
6871
});

0 commit comments

Comments
 (0)