From ba0ac01f7b5e961a5b10df26bffe5ab9217c7896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20K=C3=B6ssler?= Date: Wed, 4 Mar 2026 13:02:27 +0100 Subject: [PATCH] POC: Dynamic import protection --- library/agent/Attack.ts | 5 +- library/agent/api/Event.ts | 3 +- .../agent/hooks/instrumentation/loadHook.ts | 3 ++ library/sinks/ImportSink.test.ts | 46 +++++++++++++++++ library/sinks/ImportSink.ts | 50 +++++++++++++++++++ .../checkContextForInsecureImport.ts | 38 ++++++++++++++ .../detectInsecureImport.test.ts | 15 ++++++ .../dynamic-import/detectInsecureImport.ts | 36 +++++++++++++ 8 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 library/sinks/ImportSink.test.ts create mode 100644 library/sinks/ImportSink.ts create mode 100644 library/vulnerabilities/dynamic-import/checkContextForInsecureImport.ts create mode 100644 library/vulnerabilities/dynamic-import/detectInsecureImport.test.ts create mode 100644 library/vulnerabilities/dynamic-import/detectInsecureImport.ts diff --git a/library/agent/Attack.ts b/library/agent/Attack.ts index 48b6672a5..6f4c4c36b 100644 --- a/library/agent/Attack.ts +++ b/library/agent/Attack.ts @@ -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) { @@ -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"; } } diff --git a/library/agent/api/Event.ts b/library/agent/api/Event.ts index bc4a8181c..a58c0cae8 100644 --- a/library/agent/api/Event.ts +++ b/library/agent/api/Event.ts @@ -74,7 +74,8 @@ export type OperationKind = | "deserialize_op" | "graphql_op" | "eval_op" - | "ai_op"; + | "ai_op" + | "import_op"; type OperationStats = { kind: OperationKind; diff --git a/library/agent/hooks/instrumentation/loadHook.ts b/library/agent/hooks/instrumentation/loadHook.ts index 0fea1a277..466bf76eb 100644 --- a/library/agent/hooks/instrumentation/loadHook.ts +++ b/library/agent/hooks/instrumentation/loadHook.ts @@ -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"); @@ -22,6 +23,8 @@ export function onModuleLoad( context: Parameters[1], previousLoadResult: ReturnType ): ReturnType { + ImportSink.checkImport(path); + try { // Ignore unsupported formats, e.g. wasm, native addons or json if ( diff --git a/library/sinks/ImportSink.test.ts b/library/sinks/ImportSink.test.ts new file mode 100644 index 000000000..b24102efe --- /dev/null +++ b/library/sinks/ImportSink.test.ts @@ -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", + }); + }); +}); diff --git a/library/sinks/ImportSink.ts b/library/sinks/ImportSink.ts new file mode 100644 index 000000000..75fbeb36d --- /dev/null +++ b/library/sinks/ImportSink.ts @@ -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) { + // + } +} diff --git a/library/vulnerabilities/dynamic-import/checkContextForInsecureImport.ts b/library/vulnerabilities/dynamic-import/checkContextForInsecureImport.ts new file mode 100644 index 000000000..b49e26533 --- /dev/null +++ b/library/vulnerabilities/dynamic-import/checkContextForInsecureImport.ts @@ -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, + }; + } + } + } +} diff --git a/library/vulnerabilities/dynamic-import/detectInsecureImport.test.ts b/library/vulnerabilities/dynamic-import/detectInsecureImport.test.ts new file mode 100644 index 000000000..df3bd340f --- /dev/null +++ b/library/vulnerabilities/dynamic-import/detectInsecureImport.test.ts @@ -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); +}); diff --git a/library/vulnerabilities/dynamic-import/detectInsecureImport.ts b/library/vulnerabilities/dynamic-import/detectInsecureImport.ts new file mode 100644 index 000000000..5ab2062c3 --- /dev/null +++ b/library/vulnerabilities/dynamic-import/detectInsecureImport.ts @@ -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; +}