Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions library/helpers/extractPathStringsFromUserInputCached.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Context } from "../agent/Context";
import { SOURCES } from "../agent/Source";
import { extractStringsFromUserInput } from "./extractStringsFromUserInput";
import { sep } from "node:path";

type ReturnValue = ReturnType<typeof extractStringsFromUserInput>;

Expand All @@ -23,7 +22,9 @@ export function extractPathStringsFromUserInputCached(
// Performance optimization: only keep strings that contain a path separator
// as only those can be used for path traversal
// keeps the set smaller and speeds up `fs` and `path` operations
if (item.includes(sep)) {
// Check both separators: attackers may send backslash-only payloads on POSIX
// that the application normalizes to forward slashes before reaching the sink.
if (item.includes("/") || item.includes("\\")) {
userStrings.add(item);
}
}
Expand Down
4 changes: 2 additions & 2 deletions library/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"compilerOptions": {
"target": "ES2019",
"lib": ["ES2019", "DOM"],
"target": "ES2021",
"lib": ["ES2021", "DOM"],
"module": "commonjs",
"moduleResolution": "node",
"declaration": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,42 @@ const unsafeContext = {
},
};

t.test(
"it detects path traversal when attacker uses backslashes (POSIX bypass)",
async () => {
t.same(
checkContextForPathTraversal({
filename: "../../../../etc/hosts",
operation: "path.resolve",
context: {
cookies: {},
headers: {},
remoteAddress: "ip",
method: "GET",
url: "url",
query: {
file: "..\\..\\..\\..\\etc\\hosts",
},
body: {},
routeParams: {},
source: "express",
route: undefined,
},
}),
{
operation: "path.resolve",
kind: "path_traversal",
source: "query",
pathsToPayload: [".file"],
metadata: {
filename: "../../../../etc/hosts",
},
payload: "..\\..\\..\\..\\etc\\hosts",
}
);
}
);

t.test("it detects path traversal from route parameter", async () => {
t.same(checkContextForPathTraversal(unsafeContext), {
operation: "operation",
Expand Down
18 changes: 16 additions & 2 deletions library/vulnerabilities/path-traversal/detectPathTraversal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,27 @@ export function detectPathTraversal(
}
}

if (userInput.length > filePath.length) {
// Normalize backslashes to forward slashes on both sides for cross-platform comparison.
// Attackers may send backslash payloads that the application normalizes to forward
// slashes before reaching the sink (filePath has '/', userInput still has '\').
const normalizedFilePath = filePath.includes("\\")
? filePath.replaceAll("\\", "/")
: filePath;
const normalizedUserInput = userInput.includes("\\")
? userInput.replaceAll("\\", "/")
: userInput;

if (normalizedUserInput.length > normalizedFilePath.length) {
// We ignore cases where the user input is longer than the file path.
// Because the user input can't be part of the file path.
return false;
}

if (!filePath.toLowerCase().includes(userInput.toLowerCase())) {
if (
!normalizedFilePath
.toLowerCase()
.includes(normalizedUserInput.toLowerCase())
) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double check if the logic below can be unchanged?

// We ignore cases where the user input is not part of the file path.
return false;
}
Expand Down
Loading