Skip to content

Commit

Permalink
Check context for sensitive file access
Browse files Browse the repository at this point in the history
  • Loading branch information
timokoessler committed Dec 20, 2024
1 parent e8fcc4c commit 516340c
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 8 deletions.
3 changes: 2 additions & 1 deletion library/agent/Agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { Users } from "./Users";
import { wrapInstalledPackages } from "./wrapInstalledPackages";
import { Wrapper } from "./Wrapper";
import { isAikidoCI } from "../helpers/isAikidoCI";
import { InterceptorResultSource } from "./hooks/InterceptorResult";

type WrappedPackage = { version: string | null; supported: boolean };

Expand Down Expand Up @@ -150,7 +151,7 @@ export class Agent {
operation: string;
kind: Kind;
blocked: boolean;
source: Source;
source: InterceptorResultSource;
request: Context;
stack: string;
paths: string[];
Expand Down
2 changes: 1 addition & 1 deletion library/agent/Attack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ export function attackKindHumanName(kind: Kind) {
case "js_injection":
return "a JavaScript injection";
case "sensitive_file_access":
return "a sensitive file access";
return "access to a sensitive file";
}
}
4 changes: 2 additions & 2 deletions library/agent/api/Event.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Kind } from "../Attack";
import { Source } from "../Source";
import { InterceptorResultSource } from "../hooks/InterceptorResult";

export type AgentInfo = {
dryMode: boolean;
Expand Down Expand Up @@ -53,7 +53,7 @@ export type DetectedAttack = {
operation: string;
module: string;
blocked: boolean;
source: Source;
source: InterceptorResultSource;
path: string;
stack: string;
payload: string;
Expand Down
4 changes: 3 additions & 1 deletion library/agent/hooks/InterceptorResult.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { Kind } from "../Attack";
import { Source } from "../Source";

export type InterceptorResultSource = Source | "route";

export type InterceptorResult = {
operation: string;
kind: Kind;
source: Source;
source: InterceptorResultSource;
pathsToPayload: string[];
metadata: Record<string, string>;
payload: unknown;
Expand Down
29 changes: 29 additions & 0 deletions library/sinks/FileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,24 @@ import { Wrapper } from "../agent/Wrapper";
import { getSemverNodeVersion } from "../helpers/getNodeVersion";
import { isVersionGreaterOrEqual } from "../helpers/isVersionGreaterOrEqual";
import { checkContextForPathTraversal } from "../vulnerabilities/path-traversal/checkContextForPathTraversal";
import { checkContextForSensitiveFileAccess } from "../vulnerabilities/sensitive-file-access/checkContextForSensitiveFile";

type FileSystemFunction = {
pathsArgs: number; // The amount of arguments that are paths
sync: boolean; // Whether the function has a synchronous version (e.g. fs.accessSync)
promise: boolean; // Whether the function has a promise version (e.g. fs.promises.access)
};

const operationsToCheckForSensitiveFileAccess = [
"open",
"opendir",
"readdir",
"readFile",
"openSync",
"readdirSync",
"readFileSync",
];

export class FileSystem implements Wrapper {
private patchedPromises = false;

Expand Down Expand Up @@ -46,6 +57,24 @@ export class FileSystem implements Wrapper {
}
}

if (
args &&
args.length &&
amountOfPathArgs === 1 &&
typeof args[0] === "string" &&
operationsToCheckForSensitiveFileAccess.includes(name)
) {
const result = checkContextForSensitiveFileAccess({
filename: args[0],
operation: `fs.${name}`,
context: context,
});

if (result) {
return result;
}
}

return undefined;
}

Expand Down
16 changes: 15 additions & 1 deletion library/sources/Express.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,12 @@ export function createExpressTests(expressPackageName: string) {

app.use(newRouter);

app.use("/", express.static(__dirname + "/fixtures/public/"));
app.use(
"/",
express.static(__dirname + "/fixtures/public/", {
dotfiles: "allow",
})
);

const nestedApp = express();
nestedApp.set("trust proxy", true);
Expand Down Expand Up @@ -719,4 +724,13 @@ export function createExpressTests(expressPackageName: string) {
);
}
);

t.test("it prevents access to sensitive files", async (t) => {
const response = await request(getApp()).get("/.env.test");

t.same(JSON.parse(response.body.toString()), {
error:
"Zen has blocked access to a sensitive file: fs.open(...) originating from route.",
});
});
}
1 change: 1 addition & 0 deletions library/sources/fixtures/public/.env.test
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Testfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { Context } from "../../agent/Context";
import { InterceptorResult } from "../../agent/hooks/InterceptorResult";
import { pathToString } from "../../helpers/pathToString";
import { isSensitiveFile } from "./isSensitiveFile";
import { getMatchingPathEnding } from "./getMatchingPathEnding";

export function checkContextForSensitiveFileAccess({
filename,
context,
operation,
}: {
filename: string | URL | Buffer;
context: Context;
operation: string;
}): InterceptorResult {
const filePathString = pathToString(filename);
if (!filePathString) {
return;
}

Check warning on line 19 in library/vulnerabilities/sensitive-file-access/checkContextForSensitiveFile.ts

View check run for this annotation

Codecov / codecov/patch

library/vulnerabilities/sensitive-file-access/checkContextForSensitiveFile.ts#L18-L19

Added lines #L18 - L19 were not covered by tests

if (!context.route) {
return;
}

Check warning on line 23 in library/vulnerabilities/sensitive-file-access/checkContextForSensitiveFile.ts

View check run for this annotation

Codecov / codecov/patch

library/vulnerabilities/sensitive-file-access/checkContextForSensitiveFile.ts#L22-L23

Added lines #L22 - L23 were not covered by tests

const matchingPathEnding = getMatchingPathEnding(
context.route,
filePathString
);

if (!matchingPathEnding) {
return;
}

if (isSensitiveFile(matchingPathEnding)) {
return {
operation: operation,
kind: "sensitive_file_access",
source: "route",
pathsToPayload: ["."],
metadata: {
filename: filePathString,
},
payload: context.route,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,25 @@ t.test("is a sensitive file", async (t) => {
t.same(isSensitiveFile("/.git"), true);
t.same(isSensitiveFile("/.git/"), true);
t.same(isSensitiveFile("/.git/test"), true);
t.same(isSensitiveFile("/.aws/keys.conf"), true);
t.same(isSensitiveFile("/.ssh/config"), true);
t.same(isSensitiveFile("/.circleci/config.yml"), true);
t.same(isSensitiveFile("/.github/secrets"), true);
t.same(isSensitiveFile("/.docker/config.json"), true);
t.same(isSensitiveFile("/.env.prod"), true);
t.same(isSensitiveFile("/backend/.env-Production"), true);
t.same(isSensitiveFile("/.env.test"), true);
t.same(isSensitiveFile("/.gitlab-ci.yml"), true);
t.same(isSensitiveFile("/ci/.travis.yml"), true);
t.same(isSensitiveFile("/.idea/"), true);
t.same(isSensitiveFile("/data/db.sqlite"), true);
t.same(isSensitiveFile("/data/db.sql"), true);
t.same(isSensitiveFile("/DoCkErFiLe"), true);
t.same(isSensitiveFile("/docker-compose.yml"), true);
t.same(isSensitiveFile("/docker-compose.dev.yml"), true);
t.same(isSensitiveFile("/docker-compose.prod.yaml"), true);
t.same(isSensitiveFile("/package-lock.json"), true);
t.same(isSensitiveFile("/static/package.Json"), true);
});

t.test("is not a sensitive file", async (t) => {
Expand All @@ -17,4 +36,9 @@ t.test("is not a sensitive file", async (t) => {
t.same(isSensitiveFile("/"), false);
t.same(isSensitiveFile("/.gitabc"), false);
t.same(isSensitiveFile("/.gitabc/test"), false);
t.same(isSensitiveFile("/xenv"), false);
t.same(isSensitiveFile("/test/"), false);
t.same(isSensitiveFile("/route/abc/123"), false);
t.same(isSensitiveFile("/data/db.sqla"), false);
t.same(isSensitiveFile("/data/sql/a"), false);
});
30 changes: 28 additions & 2 deletions library/vulnerabilities/sensitive-file-access/isSensitiveFile.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
const forbiddenFileNames = [".env", ".bashrc"];
const forbiddenDirectories = [".git", ".aws"];
const forbiddenFileNames = [
"\\.env[^/]*",
"\\.bashrc",
"\\.gitlab-ci.yml",
"\\.travis.yml",
"[^/]*\\.(?:sql|sqlite|db)",
"Dockerfile",
"docker-compose(?:[^/]*)?\\.(?:yml|yaml)",

// Node.js specific
"package-lock\\.json",
"package\\.json",
"npm-shrinkwrap\\.json",
"yarn\\.lock",
"\\.npmrc",
];
const forbiddenDirectories = [
"\\.git",
"\\.aws",
"\\.ssh",
"\\.circleci",
"\\.github",
"\\.docker",
"\\.svn",
"\\.hg",
"\\.idea",
"\\.vscode",
];

const forbiddenFileNamesPattern = `(?:.*/(?:${forbiddenFileNames.join("|")}))`;
const forbiddenDirectoriesPattern = `(?:.*/(?:${forbiddenDirectories.join("|")})(?:/.*)?)`;
Expand Down

0 comments on commit 516340c

Please sign in to comment.