fix: exclude valid OMS signatures from content analysis#261
fix: exclude valid OMS signatures from content analysis#261major-security wants to merge 1 commit into
Conversation
OMS bundles necessarily contain long base64-encoded payload, signature, and certificate fields. Generic obfuscated-code checks can misclassify these fields as hidden executable content. Recognize root-level skill.oms.sig files with the minimal OMS v0.3 DSSE/in-toto structure and inventory them as oms_signature components without sending their contents through static or LLM analyzers. Preserve normal scanning for malformed, nested, oversized, or unrecognized signature files. Add a pinned real-world fixture, regression coverage, completeness accounting, and documentation. Recognition is structural only and does not verify signatures, certificates, transparency logs, or signer identity. Signed-off-by: major-security <73253177+major-security@users.noreply.github.com>
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
[Automated SkillSpector Review]
Requesting changes. The OMS false-positive goal is reasonable, but the exemption is forgeable and the report overstates coverage. A structurally plausible, unverified bundle can hide attacker-controlled content from every analyzer, while the report still claims the excluded file was fully scanned.
Please require schema-strict parsing and analyze all non-opaque/decoded content, or cryptographically verify a trusted, subject-bound bundle before exempting it. Coverage metadata must also disclose the exclusion.
| return False | ||
|
|
||
| statement = _decode_base64_json(envelope.get("payload")) | ||
| return bool( |
There was a problem hiding this comment.
Blocking: this predicate trusts only attacker-controlled shape: type strings, any nonempty base64 sig, and any dict verificationMaterial. It does not verify the signature/chain, constrain extra fields, or bind the bundle to a trusted subject. Because the caller later removes the whole file from analysis, a forged bundle can place malicious or prompt-injection content in extra verification/predicate fields and bypass both static and LLM analyzers. Require strict schema/content handling or real cryptographic verification, and add an adversarial forged-bundle regression.
| missing_components = [c for c in components if c not in file_cache and c not in excluded] | ||
| # Intentionally excluded metadata is fully accounted for by the scan even | ||
| # though analyzers do not inspect its content. | ||
| scanned_components = total_components - len(missing_components) |
There was a problem hiding this comment.
Blocking: OMS-excluded content is still counted as scanned_components, so a file inspected by no analyzer can produce 100% coverage and is_complete=true. Keep coverage honest by excluding it from the scanned count and surfacing an explicit exclusion/limitation in report metadata.
|
There is a design tradeoff encoded here that we can debate: how much of the signature validation do we want to encoded in the obfuscated code checker? Because a signature contains many b64 encoded fields, we need to exempt valid signatures from the obfuscated code checker. The PR currently validates that the the skill.oms.sig file is a valid sigstore bundle (is a valid artifiact at the top level). An OMS signature also contains file digests for each file in covered by the signature in in-toto predicates. The PR as-is doesn't currently validate that the digest payloads in the signature file match the content of the file on disk. One one hand, an attacker could put obfuscated non-digest content in these in-toto predicates. On the other hand, if they did, the signature verification would fail upon signature checking. So it's a tradeoff of how much of the signature checking logic we build into the obfuscated content checker. Alternatively, SkillSpector could call out to the OMS signature verifier. |
2 participants
OMS bundles necessarily contain long base64-encoded payload, signature, and certificate fields. Generic obfuscated-code checks can misclassify these fields as hidden executable content.
Recognize root-level skill.oms.sig files with the minimal OMS v0.3 DSSE/in-toto structure and inventory them as oms_signature components without sending their contents through static or LLM analyzers.
Preserve normal scanning for malformed, nested, oversized, or unrecognized signature files. Add a pinned real-world fixture, regression coverage, completeness accounting, and documentation.
Recognition is structural only and does not verify signatures, certificates, transparency logs, or signer identity.