Skip to content

fix: block os.system via the nt module on Windows#2535

Open
nolanchic wants to merge 1 commit into
huggingface:mainfrom
nolanchic:fix/block-nt-system-dangerous-function
Open

fix: block os.system via the nt module on Windows#2535
nolanchic wants to merge 1 commit into
huggingface:mainfrom
nolanchic:fix/block-nt-system-dangerous-function

Conversation

@nolanchic

Copy link
Copy Markdown

What does this PR do?

Closes #2232.

check_safer_result blocks dangerous functions by matching result.__module__ + "." + result.__name__ against DANGEROUS_FUNCTIONS. On Windows, os.system is implemented in the nt module, so os.system.__module__ is "nt" — not "os". Since DANGEROUS_FUNCTIONS listed os.system and posix.system but not nt.system, the check silently let os.system through on Windows when a user authorized os via additional_authorized_imports=["os"].

The default sandbox blocks os entirely, so this only affects users who explicitly authorize it — but for them it is a sandbox escape on Windows.

How

Add nt.system to DANGEROUS_FUNCTIONS, mirroring the existing posix.system entry that covers Linux/macOS.

Test

  • The existing parametrized test_vulnerability_for_all_dangerous_functions now covers nt.system on Windows. It uses pytest.importorskip, so nt.system is skipped on Linux/macOS — exactly symmetric to how posix.system is skipped on Windows.
  • Added test_check_safer_result_blocks_nt_system, which is platform-independent: it passes check_safer_result a function with __module__ == "nt" / __name__ == "system" and asserts it is blocked. This keeps the regression covered on the Linux CI runners where nt is unavailable. Fails on main, passes with this change.

check_safer_result matches each dangerous function by 'module.name'. On Windows os.system is implemented in the nt module, so its __module__ is "nt" rather than "os", and the check silently let it through when os was authorized via additional_authorized_imports. DANGEROUS_FUNCTIONS already had posix.system for Linux/macOS but was missing nt.system.

Closes huggingface#2232
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.

BUG: DANGEROUS_FUNCTIONS check does not block os.system on Windows (nt module mismatch)

1 participant