Forbid inductor::_reinterpret_tensor (as_strided heap-OOB bypass)#3079
Draft
edsavage wants to merge 9 commits into
Draft
Forbid inductor::_reinterpret_tensor (as_strided heap-OOB bypass)#3079edsavage wants to merge 9 commits into
edsavage wants to merge 9 commits into
Conversation
TorchScript runs a module's __setstate__ during torch::jit::load(), before the loaded module reaches CModelGraphValidator (which only walks the inlined forward graph). A forbidden op hidden in __setstate__ is therefore invisible to the validator and has already executed by the time validation would run. Add fixtures and a self-contained repro that demonstrate the gap: - generate_malicious_models.py: add SetStateFileReaderModel and a submodule variant (aten::from_file in __setstate__, benign forward); surface __setstate__ ops in the generator output. - test/test_setstate_load_timing.py: torch-only, build-free repro proving load-time execution (benign print probe) and op-hiding (static inspection). - test_pytorch_inference_evil_models.py: register the fixtures as regression guards for the forthcoming pre-load scan. No fix yet; this only reproduces and documents the vulnerability. Co-authored-by: Cursor <cursoragent@cursor.com>
Close the load-time execution gap: torch::jit::load runs a module's __setstate__ during deserialization, before CModelGraphValidator (which walks an already-loaded module's graph) can run. A forbidden op hidden in __setstate__ therefore executes at load time and never appears in the forward graph the validator inspects. Add CModelGraphValidator::scanSerialisedCodeForForbiddenOps, which uses a PyTorchStreamReader over the buffered archive bytes to textually scan the serialised TorchScript code (code/**/*.py, including every submodule's __setstate__) for the forbidden aten ops, WITHOUT loading the model. Main.cc runs this scan before torch::jit::load and rejects via HANDLE_FATAL, so no model code executes. Expose the buffered bytes via CBufferedIStreamAdapter::buffer(). This is a defense-in-depth textual check targeted at the curated forbidden set (from_file, as_strided, save); the post-load allowlist validation is unchanged, and seccomp remains the syscall backstop. Unit tests cover both setstate fixtures, forward-graph attacks, a benign model (no false positive), and malformed input; the binary integration test now expects the setstate models to be rejected with the forbidden-operations message. Co-authored-by: Cursor <cursoragent@cursor.com>
…l-cpp into security/setstate-load-timing
Address Copilot review: the benign probe must load cleanly for the marker to prove load-time execution. Require returncode == 0 and that LOAD_RETURNED was printed, and print returncode + stderr tail on failure, so a crash that happens to print the marker first is no longer a false positive. Co-authored-by: Cursor <cursoragent@cursor.com>
Close the heap OOB bypass reported in elastic/security#12242: TorchInductor's _reinterpret_tensor takes an unchecked storage offset and enables the same class of OOB read/write previously mitigated by forbidding aten::as_strided. - Add inductor::_reinterpret_tensor to FORBIDDEN_OPERATIONS (fail-fast). - Extend the pre-load serialised-code scan to match ops.inductor._reinterpret_tensor( so __setstate__ payloads are rejected before torch::jit::load(). - Add OOB-read, OOB-write, and setstate fixtures + unit/integration tests. Verified against a built pytorch_inference: with validation on, models are rejected; with --skipModelValidation, load succeeds and Python forward of the same PoC aborts (SIGABRT), confirming the upstream memory bug. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Collapse the inductor:: branch condition onto one line to satisfy clang-format 5.0.1 (CI check_style). Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening for elastic/security#12242 (HackerOne): after
aten::as_stridedwas forbidden, a malicious TorchScript model can still get the same class of unchecked storage-offset OOB heap read/write viatorch.ops.inductor._reinterpret_tensor→ TorchScript opinductor::_reinterpret_tensor(serialised asops.inductor._reinterpret_tensor(...)).Verification (local, built
pytorch_inference)Against the Mar-30 distribution binary with graph validation:
Unrecognised operations: inductor::_reinterpret_tensor(not yet in FORBIDDEN on that build)--skipModelValidation.pt+torch.jit.load+forward()in PythonWith this PR, the post-load path fail-fasts as forbidden (not merely unrecognised), and the pre-load scan rejects before
torch::jit::load()(including__setstate__fixtures).Changes
CSupportedOperations::FORBIDDEN_OPERATIONS: addinductor::_reinterpret_tensor.ops.inductor._reinterpret_tensor(in serialisedcode/**/*.py.__setstate__variants + generator entries.test_pytorch_inference_evil_models.pyregistrations.Stacking
This branch is stacked on #3078 (pre-load
__setstate__scan). The PR diff vsmaintherefore includes those commits as well. #3078 should be merged first, then rebasing this; or review/merge as the combined security hardening.Test plan
e5_with_norm.ptstill clean.testPreLoadScanDetectsReinterpret*/testMaliciousReinterpretTensorRejectedPostLoadgreen.Made with Cursor