Skip to content
Draft
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: 4 additions & 1 deletion library/agent/Attack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ export type Kind =
| "path_traversal"
| "ssrf"
| "stored_ssrf"
| "code_injection";
| "code_injection"
| "insecure_import";

export function attackKindHumanName(kind: Kind) {
switch (kind) {
Expand All @@ -23,5 +24,7 @@ export function attackKindHumanName(kind: Kind) {
return "a stored server-side request forgery";
case "code_injection":
return "a JavaScript injection";
case "insecure_import":
return "an insecure import";
}
}
3 changes: 2 additions & 1 deletion library/agent/api/Event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export type OperationKind =
| "deserialize_op"
| "graphql_op"
| "eval_op"
| "ai_op";
| "ai_op"
| "import_op";

type OperationStats = {
kind: OperationKind;
Expand Down
3 changes: 3 additions & 0 deletions library/agent/hooks/instrumentation/loadHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { getInstance } from "../../AgentSingleton";
import { syncBuiltinESMExports } from "module";
import { getBuiltinModuleWithoutPatching } from "./processGetBuiltin";
import { wrapBuiltinExports } from "./wrapBuiltinExports";
import { ImportSink } from "../../../sinks/ImportSink";

const builtinPatchedSymbol = Symbol("zen.instrumentation.builtin.patched");

Expand All @@ -22,6 +23,8 @@ export function onModuleLoad(
context: Parameters<LoadFunction>[1],
previousLoadResult: ReturnType<LoadFunction>
): ReturnType<LoadFunction> {
ImportSink.checkImport(path);

try {
// Ignore unsupported formats, e.g. wasm, native addons or json
if (
Expand Down
46 changes: 46 additions & 0 deletions library/sinks/ImportSink.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import * as t from "tap";
import { startTestAgent } from "../helpers/startTestAgent";
import { runWithContext, type Context } from "../agent/Context";

t.before(async () => {
startTestAgent({
block: true,
wrappers: [],
rewrite: {},
});
});

function getTestContext(testStr: string): Context {
return {
remoteAddress: "::1",
method: "POST",
url: "http://localhost:4000/api/test",
query: {},
headers: {
"content-type": "application/json",
},
body: {
test: testStr,
},
cookies: {},
routeParams: {},
source: "hono",
route: "/api/test",
};
}

t.test("it works", async (t) => {
await import("http");
require("http2");

runWithContext(getTestContext("child_process"), () => {
const error = t.throws(() => {
require("child_process");
});

t.match(error, {
message:
"Zen has blocked an insecure import: import/require(...) originating from body.test",
});
});
});
50 changes: 50 additions & 0 deletions library/sinks/ImportSink.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { getInstance } from "../agent/AgentSingleton";
import { type Context, getContext } from "../agent/Context";
import { Hooks } from "../agent/hooks/Hooks";
import { InterceptorResult } from "../agent/hooks/InterceptorResult";
import { inspectArgs } from "../agent/hooks/wrapExport";
import { Wrapper } from "../agent/Wrapper";
import { checkContextForInsecureImport } from "../vulnerabilities/dynamic-import/checkContextForInsecureImport";

export class ImportSink implements Wrapper {
private static inspectImport(
args: [string],
context: Context
): InterceptorResult {
const [specifier] = args;

return checkContextForInsecureImport({
specifier,
context,
});
}

static checkImport(specifier: string) {
const agent = getInstance();
if (!agent) {
return undefined;
}

const context = getContext();
if (!context) {
return undefined;
}

inspectArgs(
[specifier],
(args) => this.inspectImport(args as [string], context),
context,
agent,
{
name: "import/require",
type: "global",
},
"import/require",
"import_op"
);
}

wrap(_: Hooks) {
//
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import type { Context } from "../../agent/Context";
import { InterceptorResult } from "../../agent/hooks/InterceptorResult";
import { getPathsToPayload } from "../../helpers/attackPath";
import { extractStringsFromUserInputCached } from "../../helpers/extractStringsFromUserInputCached";
import { getSourceForUserString } from "../../helpers/getSourceForUserString";
import { detectInsecureImport } from "./detectInsecureImport";

export function checkContextForInsecureImport({
specifier,
context,
}: {
specifier: string;
context: Context;
}): InterceptorResult {
// Todo check with and without node: prefix
if (specifier.startsWith("node:")) {
specifier = specifier.slice(5);
}

for (const str of extractStringsFromUserInputCached(context)) {
if (detectInsecureImport(specifier, str)) {
const source = getSourceForUserString(context, str);

if (source) {
return {
operation: "import/require",
kind: "insecure_import",
source: source,
pathsToPayload: getPathsToPayload(str, context[source]),
metadata: {
specifier,
},
payload: str,
};
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import * as t from "tap";
import { detectInsecureImport } from "./detectInsecureImport";

t.test("it detects insecure imports", async (t) => {
t.equal(detectInsecureImport("http", "http"), true);
t.equal(detectInsecureImport("../test.js", "../test.js"), true);
t.equal(detectInsecureImport("../test.js", "../test"), true);
t.equal(detectInsecureImport("/tmp/eval.js", "/tmp/eval.js"), true);

t.equal(detectInsecureImport("http", "ht"), false);
t.equal(detectInsecureImport("http", "https"), false);
t.equal(detectInsecureImport("a", "a"), false);
t.equal(detectInsecureImport("abc", "xyz"), false);
t.equal(detectInsecureImport("/tmp/eval.js", "eval.js"), false);
});
36 changes: 36 additions & 0 deletions library/vulnerabilities/dynamic-import/detectInsecureImport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { containsUnsafePathParts } from "../path-traversal/containsUnsafePathParts";

export function detectInsecureImport(
specifier: string,
userInput: string
): boolean {
if (userInput.length <= 1) {
// We ignore single characters since they don't pose a big threat.
return false;
}

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

if (!specifier.includes(userInput)) {
// We ignore cases where the user input is not part of the specifier.
return false;
}

if (userInput === specifier) {
return true;
}

// Todo: Support file:// urls (../ already resolved)
if (
containsUnsafePathParts(specifier) &&
containsUnsafePathParts(userInput)
) {
return true;
}

return false;
}
Loading