Skip to content

Commit 84ea85b

Browse files
committed
Fix unsafe path start on Windows
1 parent 0a755f0 commit 84ea85b

File tree

4 files changed

+48
-24
lines changed

4 files changed

+48
-24
lines changed

library/sinks/FileSystem.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ t.test("it works", async (t) => {
100100
if (!isWindows) {
101101
rename(new URL("file:///test123.txt"), "test2.txt", (err) => {});
102102
} else {
103-
rename(new URL("file:///c:/test123.txt"), "test2.txt", (err) => {});
103+
rename(new URL("file:///x:/test123.txt"), "test2.txt", (err) => {});
104104
}
105105
rename(Buffer.from("./test123.txt"), "test2.txt", (err) => {});
106106
};
@@ -184,7 +184,7 @@ t.test("it works", async (t) => {
184184
throws(
185185
() =>
186186
rename(
187-
new URL("file:///C:/../test.txt"),
187+
new URL("file:///x:/../test.txt"),
188188
"../test2.txt",
189189
(err) => {}
190190
),
@@ -194,7 +194,7 @@ t.test("it works", async (t) => {
194194
throws(
195195
() =>
196196
rename(
197-
new URL("file:///C:/./../test.txt"),
197+
new URL("file:///x:/./../test.txt"),
198198
"../test2.txt",
199199
(err) => {}
200200
),
@@ -204,7 +204,7 @@ t.test("it works", async (t) => {
204204
throws(
205205
() =>
206206
rename(
207-
new URL("file:///C:/../../test.txt"),
207+
new URL("file:///X:/../../test.txt"),
208208
"../test2.txt",
209209
(err) => {}
210210
),
@@ -244,7 +244,7 @@ t.test("it works", async (t) => {
244244
rename(new URL("file:///../../test.txt"), "../test2.txt", (err) => {});
245245
} else {
246246
rename(
247-
new URL("file:/C://../../test.txt"),
247+
new URL("file:/x://../../test.txt"),
248248
"../test2.txt",
249249
(err) => {}
250250
);

library/sinks/Path.test.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ t.test("it works", async (t) => {
4747
if (!isWindows) {
4848
t.same(join("/app", "/etc/data"), resolve("/app/etc/data"));
4949
} else {
50-
t.same(join("c:/app", "/etc/data"), resolve("c:/app/etc/data"));
50+
t.same(join("x:/app", "/etc/data"), resolve("x:/app/etc/data"));
5151
}
5252
}
5353

@@ -98,8 +98,8 @@ t.test("it works", async (t) => {
9898
safeCalls();
9999
});
100100

101-
runWithContext(unsafeAbsoluteContext, () => {
102-
if (!isWindows) {
101+
if (!isWindows) {
102+
runWithContext(unsafeAbsoluteContext, () => {
103103
t.throws(
104104
() => join("/etc/", "test.txt"),
105105
"Zen has blocked a Path traversal: fs.join(...) originating from body.file.matches"
@@ -109,11 +109,21 @@ t.test("it works", async (t) => {
109109
() => resolve("/etc/some_directory", "test.txt"),
110110
"Zen has blocked a Path traversal: fs.resolve(...) originating from body.file.matches"
111111
);
112-
} else {
113-
t.throws(
114-
() => join("C:/etc/", "test.txt"),
115-
"Zen has blocked a Path traversal: fs.join(...) originating from body.file.matches"
116-
);
117-
}
118-
});
112+
});
113+
} else {
114+
runWithContext(
115+
{
116+
...unsafeAbsoluteContext,
117+
...{
118+
body: { file: { matches: "X:/etc/" } },
119+
},
120+
},
121+
() => {
122+
t.throws(
123+
() => resolve("X:/etc/", "test.txt"),
124+
"Zen has blocked a Path traversal: fs.join(...) originating from body.file.matches"
125+
);
126+
}
127+
);
128+
}
119129
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,6 @@ t.test(
126126
"windows drive letter",
127127
{ skip: process.platform !== "win32" ? "Windows only" : false },
128128
async () => {
129-
t.same(detectPathTraversal("C:\\file.txt", "C:\\"), true);
129+
t.same(detectPathTraversal("X:\\file.txt", "X:\\"), true);
130130
}
131131
);

library/vulnerabilities/path-traversal/unsafePathStart.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const linuxRootFolders = [
2222
"/usr/",
2323
"/var/",
2424
];
25-
const dangerousPathStarts = [...linuxRootFolders, "c:/", "c:\\"];
2625

2726
export function startsWithUnsafePath(filePath: string, userInput: string) {
2827
// Check if path is relative (not absolute or drive letter path)
@@ -38,13 +37,28 @@ export function startsWithUnsafePath(filePath: string, userInput: string) {
3837

3938
const normalizedPath = origResolve(filePath).toLowerCase();
4039
const normalizedUserInput = origResolve(userInput).toLowerCase();
41-
for (const dangerousStart of dangerousPathStarts) {
42-
if (
43-
normalizedPath.startsWith(dangerousStart) &&
44-
normalizedPath.startsWith(normalizedUserInput)
45-
) {
46-
return true;
47-
}
40+
41+
return isDangerous(normalizedPath, normalizedUserInput);
42+
}
43+
44+
function isDangerous(normalizedPath: string, normalizedUserInput: string) {
45+
// Check if normalizedPath starts with normalizedUserInput
46+
if (!normalizedPath.startsWith(normalizedUserInput)) {
47+
return false;
48+
}
49+
50+
const foundLinuxRoot = linuxRootFolders.find((folder) =>
51+
normalizedPath.startsWith(folder)
52+
);
53+
54+
if (foundLinuxRoot) {
55+
return true;
4856
}
57+
58+
// Check for windows drive letter
59+
if (/^[a-z]:(\\|\/)/i.test(normalizedPath)) {
60+
return true;
61+
}
62+
4963
return false;
5064
}

0 commit comments

Comments
 (0)