Skip to content

fix(behavioral): detect builtins.* and importlib.import_module sink evasions#180

Open
zied-jlassi wants to merge 1 commit into
NVIDIA:mainfrom
zied-jlassi:fix/builtins-importlib-sink-evasion
Open

fix(behavioral): detect builtins.* and importlib.import_module sink evasions#180
zied-jlassi wants to merge 1 commit into
NVIDIA:mainfrom
zied-jlassi:fix/builtins-importlib-sink-evasion

Conversation

@zied-jlassi

Copy link
Copy Markdown
Contributor

Summary

Complementary to #166 (the getattr branch). #166 closed reflective getattr(os,'system') evasion; this PR closes the disjoint import / builtins / importlib branch, which #166 does not cover.

Problem

The import-alias normalization rewrites from builtins import exec and import builtins; builtins.exec(...) to the qualified builtins.exec, but the analyzers match dangerous builtins by bare name (call_name == "exec", _EXEC_SINKS) — so these get missed while plain os.system is caught. Separately, importlib.import_module('os').system(...) (and the subprocess family, and bare-imported import_module) is never matched: only __import__ is.

Fix

  1. Collapse builtins.<x> back to the bare builtin at the alias-normalization chokepoint — builtins.exec is exec, so this is semantically exact and reuses the existing checks. Covers exec/eval/compile/__import__.
  2. Add resolve_dynamic_import_call (sibling of __import__) resolving importlib.import_module('mod').attr to the canonical mod.attr sink, re-entering the existing os./subprocess. ladders. Covers the subprocess family too.

0 new dependencies, backward-compatible, composes with #166 (no double-fire; getattr(os,'system') stays a single AST9).

Tests

Added TestBuiltinsImportEvasion, TestImportlibDynamicChainEvasion (behavioral_ast) and TestBuiltinsImportlibSinkEvasion (taint): every evasion form detected; baseline still detected; FP-neighbors (from mymod import exec_helper, import_module('json').loads) stay clean. Full behavioral analyzer suite green (104/104 incl. #166's AST9 tests).

Prepared with AI assistance; every line was reviewed and validated by the author.

…vasions

The import-alias normalization rewrites `from builtins import exec` and `import builtins; builtins.exec(...)` to the qualified `builtins.exec`, which the bare-name sink checks (call_name == "exec", _EXEC_SINKS) then miss. Collapse `builtins.<x>` back to the bare builtin so these re-enter the existing detection, covering exec/eval/compile/__import__. Add a dynamic-import resolver so `importlib.import_module('os').system(...)` (sibling of __import__) resolves to os.system / the subprocess family and hits the sink ladders.

Baseline detection unchanged; composes with the getattr branch (NVIDIA#166) with no double-fire; FP-neighbors stay clean.

Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Automated SkillSpector Review]

Approved.

High-quality anti-evasion work closing two disjoint bypasses: from builtins import exec / builtins.exec(...) (collapsed to the bare builtin via _strip_builtins_prefix — semantically exact since builtins.exec is exec) and importlib.import_module('os').system(...) (resolved to os.system via resolve_dynamic_import_call). Both are wired as fallbacks only when primary resolution returns None, so they don't double-fire with the getattr branch (#166). Resolution stays precise (literal-only module/attr), and FP-neighbors (from mymod import exec_helper, import_module('json').loads) are explicitly tested. 15 new tests across AST + taint. Directly strengthens detection — no blockers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants