Skip to content

Fix the type of exception thrown when decoding bad headers#124411

Open
bartonjs wants to merge 1 commit intodotnet:mainfrom
bartonjs:cose_exceptions
Open

Fix the type of exception thrown when decoding bad headers#124411
bartonjs wants to merge 1 commit intodotnet:mainfrom
bartonjs:cose_exceptions

Conversation

@bartonjs
Copy link
Member

CoseMessage's Decode routines say that any failure is a CryptographicException, but some of the validation failures leak out ArgumentException from the validation in CoseHeaderMap.

CoseMessage's Decode routines say that any failure is a CryptographicException,
but some of the validation failures leak out ArgumentException from the validation in CoseHeaderMap.
@bartonjs bartonjs added this to the 11.0.0 milestone Feb 14, 2026
@bartonjs bartonjs self-assigned this Feb 14, 2026
Copilot AI review requested due to automatic review settings February 14, 2026 00:04
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where CoseMessage.DecodeSign1 and CoseMessage.DecodeMultiSign methods would leak ArgumentException from internal validation in CoseHeaderMap, breaking the documented contract that decode failures should throw CryptographicException. The fix wraps these exceptions and also adds support for indefinite-length CBOR arrays in critical headers validation.

Changes:

  • Modified exception handling in DecodeBucket to wrap validation ArgumentException into CryptographicException
  • Refactored CBOR array/map parsing to support both definite and indefinite-length encoding
  • Added comprehensive test coverage for critical header validation with both encoding styles

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseMessage.cs Added exception wrapping in DecodeBucket and refactored to support indefinite-length CBOR collections
src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeSign1.cs Added tests verifying CryptographicException is thrown for critical header validation errors
src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeMultiSign.cs Added tests verifying CryptographicException is thrown for critical header validation errors in multi-sign messages
src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.Sign.CustomHeaderMaps.cs Extended existing tests to cover both definite and indefinite-length critical header arrays

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant