Skip to content

Commit e256302

Browse files
timokoesslertimokoessler
committed
Merge branch 'main' into windows-support
2 parents 24a6482 + 916e44a commit e256302

File tree

5 files changed

+271
-2
lines changed

5 files changed

+271
-2
lines changed

‎library/sinks/FileSystem.test.ts‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,86 @@ t.test("it works", async (t) => {
192192
rename(new URL("file:/C://../../test.txt"), "../test2.txt", () => {}),
193193
"Zen has blocked a path traversal attack: fs.rename(...) originating from body.file.matches"
194194
);
195+
196+
runWithContext(
197+
{
198+
remoteAddress: "::1",
199+
method: "POST",
200+
url: "http://localhost:4000",
201+
query: {
202+
q: ".\t./etc/passwd",
203+
},
204+
headers: {},
205+
body: {},
206+
cookies: {},
207+
routeParams: {},
208+
source: "express",
209+
route: "/posts/:id",
210+
},
211+
() => {
212+
throws(
213+
() =>
214+
rename(
215+
new URL("file://D:/.\t./etc/passwd"),
216+
"../test123.txt",
217+
() => {}
218+
),
219+
"Zen has blocked a path traversal attack: fs.rename(...) originating from query.q"
220+
);
221+
}
222+
);
223+
224+
runWithContext(
225+
{
226+
remoteAddress: "::1",
227+
method: "POST",
228+
url: "http://localhost:4000",
229+
query: {
230+
q: "test/test.txt",
231+
},
232+
headers: {},
233+
body: {},
234+
cookies: {},
235+
routeParams: {},
236+
source: "express",
237+
route: "/posts/:id",
238+
},
239+
() => {
240+
rename(
241+
new URL("file://D:/test/test.txt"),
242+
"../test123.txt",
243+
() => {}
244+
);
245+
}
246+
);
247+
248+
runWithContext(
249+
{
250+
remoteAddress: "::1",
251+
method: "POST",
252+
url: "http://localhost:4000",
253+
query: {
254+
q: ".\t\t./etc/passwd",
255+
},
256+
headers: {},
257+
body: {},
258+
cookies: {},
259+
routeParams: {},
260+
source: "express",
261+
route: "/posts/:id",
262+
},
263+
() => {
264+
throws(
265+
() =>
266+
rename(
267+
new URL("file://D:/.\t\t./etc/passwd"),
268+
"../test123.txt",
269+
() => {}
270+
),
271+
"Zen has blocked a path traversal attack: fs.rename(...) originating from query.q"
272+
);
273+
}
274+
);
195275
} else {
196276
throws(
197277
() => rename(new URL("file:///../test.txt"), "../test2.txt", () => {}),
@@ -215,6 +295,82 @@ t.test("it works", async (t) => {
215295
() => rename(Buffer.from("../test.txt"), "../test2.txt", () => {}),
216296
"Zen has blocked a path traversal attack: fs.rename(...) originating from body.file.matches"
217297
);
298+
299+
runWithContext(
300+
{
301+
remoteAddress: "::1",
302+
method: "POST",
303+
url: "http://localhost:4000",
304+
query: {
305+
q: ".\t./etc/passwd",
306+
},
307+
headers: {},
308+
body: {},
309+
cookies: {},
310+
routeParams: {},
311+
source: "express",
312+
route: "/posts/:id",
313+
},
314+
() => {
315+
throws(
316+
() =>
317+
rename(
318+
new URL("file:///.\t./etc/passwd"),
319+
"../test123.txt",
320+
() => {}
321+
),
322+
"Zen has blocked a path traversal attack: fs.rename(...) originating from query.q"
323+
);
324+
}
325+
);
326+
327+
runWithContext(
328+
{
329+
remoteAddress: "::1",
330+
method: "POST",
331+
url: "http://localhost:4000",
332+
query: {
333+
q: "test/test.txt",
334+
},
335+
headers: {},
336+
body: {},
337+
cookies: {},
338+
routeParams: {},
339+
source: "express",
340+
route: "/posts/:id",
341+
},
342+
() => {
343+
rename(new URL("file:///test/test.txt"), "../test123.txt", () => {});
344+
}
345+
);
346+
347+
runWithContext(
348+
{
349+
remoteAddress: "::1",
350+
method: "POST",
351+
url: "http://localhost:4000",
352+
query: {
353+
q: ".\t\t./etc/passwd",
354+
},
355+
headers: {},
356+
body: {},
357+
cookies: {},
358+
routeParams: {},
359+
source: "express",
360+
route: "/posts/:id",
361+
},
362+
() => {
363+
throws(
364+
() =>
365+
rename(
366+
new URL("file:///.\t\t./etc/passwd"),
367+
"../test123.txt",
368+
() => {}
369+
),
370+
"Zen has blocked a path traversal attack: fs.rename(...) originating from query.q"
371+
);
372+
}
373+
);
218374
});
219375

220376
if (!isWindows) {

‎library/vulnerabilities/path-traversal/checkContextForPathTraversal.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,40 @@ t.test("it ignores invalid filename type", async () => {
141141
undefined
142142
);
143143
});
144+
145+
t.test("it works with control characters removed by new URL", async (t) => {
146+
const queries = [".\t./etc/passwd", ".\n./etc/passwd", ".\r./etc/passwd"];
147+
148+
for (const query of queries) {
149+
t.same(
150+
checkContextForPathTraversal({
151+
filename: new URL(`file:///test/${query}`),
152+
operation: "operation",
153+
context: {
154+
cookies: {},
155+
headers: {},
156+
remoteAddress: "ip",
157+
method: "POST",
158+
url: "url",
159+
query: {
160+
q: query,
161+
},
162+
body: {},
163+
routeParams: {},
164+
source: "express",
165+
route: undefined,
166+
},
167+
}),
168+
{
169+
operation: "operation",
170+
kind: "path_traversal",
171+
source: "query",
172+
pathsToPayload: [".q"],
173+
metadata: {
174+
filename: "/etc/passwd",
175+
},
176+
payload: query,
177+
}
178+
);
179+
}
180+
});

‎library/vulnerabilities/path-traversal/containsUnsafePathParts.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import * as t from "tap";
2+
import {
3+
containsUnsafePathParts,
4+
containsUnsafePathPartsUrl,
5+
} from "./containsUnsafePathParts";
6+
import { fileURLToPath } from "url";
7+
8+
t.test("not a dangerous path", async () => {
9+
t.same(containsUnsafePathParts("test.txt"), false);
10+
t.same(containsUnsafePathParts("/var/www/test.txt"), false);
11+
t.same(containsUnsafePathParts("./test.txt"), false);
12+
t.same(containsUnsafePathParts("test.txt/"), false);
13+
});
14+
15+
t.test("it detects dangerous path parts", async () => {
16+
t.same(containsUnsafePathParts("../test.txt"), true);
17+
t.same(containsUnsafePathParts("..\\test.txt"), true);
18+
t.same(containsUnsafePathParts("../../test.txt"), true);
19+
t.same(containsUnsafePathParts("..\\..\\test.txt"), true);
20+
t.same(containsUnsafePathParts("../../../../test.txt"), true);
21+
t.same(containsUnsafePathParts("..\\..\\..\\..\\test.txt"), true);
22+
t.same(containsUnsafePathParts("/test/../test.txt"), true);
23+
t.same(containsUnsafePathParts("/test/..\\test.txt"), true);
24+
});
25+
26+
t.test(
27+
"containsUnsafePathParts does not detect with control characters",
28+
async () => {
29+
t.same(containsUnsafePathParts("file:///.\t./test.txt"), false);
30+
t.same(containsUnsafePathParts("file:///.\n./test.txt"), false);
31+
t.same(containsUnsafePathParts("file:///.\r./test.txt"), false);
32+
}
33+
);
34+
35+
t.test("it detects dangerous path parts for URLs", async () => {
36+
t.same(containsUnsafePathPartsUrl("/../test.txt"), true);
37+
t.same(containsUnsafePathPartsUrl("/..\\test.txt"), true);
38+
t.same(containsUnsafePathPartsUrl("file:///../test.txt"), true);
39+
t.same(containsUnsafePathPartsUrl("file://..\\test.txt"), true);
40+
t.same(containsUnsafePathPartsUrl("file:///.\t./test.txt"), true);
41+
t.same(containsUnsafePathPartsUrl("file://.\n./test.txt"), true);
42+
t.same(containsUnsafePathPartsUrl("file://.\r./test.txt"), true);
43+
t.same(containsUnsafePathPartsUrl("file:///.\t\t./test.txt"), true);
44+
t.same(containsUnsafePathPartsUrl("file:///.\t\n./test.txt"), true);
45+
});
46+
47+
t.test("it only removes some chars from the URL", async () => {
48+
t.same(fileURLToPath("file:///.\t./test.txt"), "/test.txt");
49+
t.same(fileURLToPath("file:///.\n./test.txt"), "/test.txt");
50+
t.same(fileURLToPath("file:///.\r./test.txt"), "/test.txt");
51+
t.same(fileURLToPath("file:///.\0./test.txt"), "/.\0./test.txt");
52+
t.same(fileURLToPath("file:///.\u0000./test.txt"), "/.\u0000./test.txt");
53+
t.same(fileURLToPath("file:///.\v./test.txt"), "/.\v./test.txt");
54+
t.same(fileURLToPath("file:///.\f./test.txt"), "/.\f./test.txt");
55+
t.same(fileURLToPath("file:///.\b./test.txt"), "/.\b./test.txt");
56+
t.same(fileURLToPath("file:///.\t\t./test.txt"), "/test.txt");
57+
t.same(fileURLToPath("file:///.\t\n./test.txt"), "/test.txt");
58+
});

‎library/vulnerabilities/path-traversal/containsUnsafePathParts.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,17 @@ export function containsUnsafePathParts(filePath: string) {
99

1010
return false;
1111
}
12+
13+
/**
14+
* This function is used for urls, because they can contain a TAB, carriage return or line feed that is silently removed by the URL constructor.
15+
*
16+
* The WHATWG URL spec defines the following:
17+
* - Remove all ASCII tab or newline from input.
18+
* - An ASCII tab or newline is U+0009 TAB, U+000A LF, or U+000D CR.
19+
*
20+
* See https://url.spec.whatwg.org/#url-parsing
21+
*/
22+
export function containsUnsafePathPartsUrl(filePath: string) {
23+
const normalized = filePath.replace(/[\t\n\r]/g, "");
24+
return containsUnsafePathParts(normalized);
25+
}

‎library/vulnerabilities/path-traversal/detectPathTraversal.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { isAbsolute } from "path";
22
import { isWindows } from "../../helpers/isWindows";
3-
import { containsUnsafePathParts } from "./containsUnsafePathParts";
3+
import {
4+
containsUnsafePathParts,
5+
containsUnsafePathPartsUrl,
6+
} from "./containsUnsafePathParts";
47
import { startsWithUnsafePath } from "./unsafePathStart";
58
import { fileURLToPath } from "url";
69

@@ -20,7 +23,8 @@ export function detectPathTraversal(
2023
// The normal check for relative path traversal will fail in this case, because transformed path does not contain ../.
2124
// For absolute path traversal, we dont need to check the transformed path, because it will always start with /.
2225
// Also /./ is checked by normal absolute path traversal check (if #219 is merged)
23-
if (isUrl && containsUnsafePathParts(userInput)) {
26+
// Use containsUnsafePathPartsUrl, because urls can contain a TAB, carriage return or line feed that is silently removed by the URL constructor.
27+
if (isUrl && containsUnsafePathPartsUrl(userInput)) {
2428
const filePathFromUrl = parseAsFileUrl(userInput);
2529
if (filePathFromUrl && filePath.includes(filePathFromUrl)) {
2630
return true;

0 commit comments

Comments
 (0)