reject signed manifest paths that escape the model directory - #647
Open
Support-itecz wants to merge 1 commit into
Open
reject signed manifest paths that escape the model directory#647Support-itecz wants to merge 1 commit into
Support-itecz wants to merge 1 commit into
Conversation
Contributor
|
You should put your description above into the patch description. |
With ignore_unsigned_files enabled, verifying.Config.verify builds the list of files to hash by joining each resource identifier from the signed manifest onto the model path, with no check that the result stays inside the model directory. A manifest whose resource name is ../secret.txt (or ../../etc/hosts) makes the verifier open and hash a file outside the model root. Anyone whose signature the configuration accepts (for example a keyless identity verified against) can therefore steer reads outside the intended directory during verification. Resolve each joined path and reject any that escape the resolved model root before hashing, matching the containment approach already used for ignore-paths in _cli._resolve_ignore_paths. Keeping the check in the verifier means every signing method benefits without callers needing their own guard. Add a key-signed regression test that crafts a manifest with a .. path and confirms verification now raises, plus a positive case that in-model paths still verify. Signed-off-by: Itecz Solution <support@itecz.au>
Support-itecz
force-pushed
the
verify-path-containment
branch
from
July 16, 2026 06:23
34e7e00 to
5a64600
Compare
Contributor
Author
|
Done, moved the description into the commit message. |
Member
|
Let's wait on this one until we have all conformance tests in https://github.com/ossf/model-signing-spec and validate the spec there. |
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
With
ignore_unsigned_filesenabled,verifying.Config.verifybuilds the list of files to hash by joining each resource identifier from the signed manifest onto the model path, with no check that the result stays inside the model directory. A manifest whose resource name is../secret.txt(or../../etc/hosts) makes the verifier open and hash a file outside the model root. Anyone whose signature your configuration accepts (for example a keyless identity you verify against) can therefore steer reads outside the intended directory during verification.This resolves each joined path and rejects any that escape the model root before hashing, matching the containment approach already used for ignore-paths in
_cli._resolve_ignore_paths. Keeping the check in the verifier means every signing method benefits without callers needing their own guard. Added a key-signed regression test that crafts a manifest with a..path and confirms verification now raises, plus a positive case that in-model paths still verify. To reproduce before the fix: sign a manifest containing a../resource, thenverify(..., ignore_unsigned_files=True)reads the outside file and can pass.Checklist