Fix OverflowException in COSE decoder for oversized integer header labels

[Copilot]

Description

CoseMessage.DecodeSign1 throws raw OverflowException instead of CryptographicException when CBOR header labels or critical header values contain integers that overflow Int32. This affects two decode paths: header map label parsing and critical header array element parsing.

Changes

• CborReaderExtensions.ReadInt32ForCrypto — Extension method on CborReader that wraps ReadInt32(), catching OverflowException and rethrowing as CryptographicException with the original as InnerException
• CoseMessage.DecodeBucket — Uses reader.ReadInt32ForCrypto() for header label reads; preserves InnerException when lifting ArgumentException to CryptographicException
• CoseMessage.MissingCriticalHeaders — Uses reader.ReadInt32ForCrypto() for crit header array elements
• CoseHeaderMap.ValidateInsertion — Uses reader.ReadInt32ForCrypto() for crit header integer reads, consistent with all other CBOR integer decode paths
• Regression tests — Two tests covering both overflow paths, asserting CryptographicException with OverflowException inner

// Before: OverflowException escapes to caller
CoseMessage.DecodeSign1(malformedData); // throws OverflowException

// After: properly wrapped
CoseMessage.DecodeSign1(malformedData); // throws CryptographicException (inner: OverflowException)

---

Fixes #126036

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Fixes COSE decode behavior so oversized CBOR integer header labels / critical header entries no longer leak raw OverflowException and are surfaced as CryptographicException (with the original OverflowException preserved as InnerException), matching the library’s error-shaping expectations for malformed inputs.

Changes:

• Added CborReader.ReadInt32ForCrypto() extension to wrap ReadInt32() overflow into CryptographicException.
• Updated COSE decode paths (DecodeBucket, MissingCriticalHeaders) to use the new helper and to preserve inner exceptions when lifting ArgumentException to CryptographicException.
• Updated header value validation (CoseHeaderMap.ValidateInsertion) to treat OverflowException as a CBOR/value validation failure and wrap it consistently.
• Added regression tests for both overflow paths.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeSign1.cs	Adds regression coverage asserting CryptographicException with OverflowException inner for oversized label / crit element inputs.
src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseMessage.cs	Uses overflow-wrapping int reads during header-label and crit-header processing; preserves inner exceptions when rethrowing.
src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseHeaderMap.cs	Wraps OverflowException from CBOR validation as ArgumentException (consistent with other invalid CBOR/value errors).
src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CborReaderExtensions.cs	Introduces ReadInt32ForCrypto() to convert OverflowException into CryptographicException with inner exception preserved.
src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj	Includes the new CborReaderExtensions.cs in the build.

[github-actions]

🤖 Copilot Code Review — PR #126095

Note

This review was generated by GitHub Copilot and should be treated as advisory. A human reviewer should make the final decision.

Holistic Assessment

Motivation: The PR fixes a real bug where CborReader.ReadInt32() throws OverflowException (via its checked cast) when a COSE header label integer exceeds Int32 range, and that exception escapes uncaught because the existing catch filters only handle CborContentException and InvalidOperationException. This is a legitimate correctness issue — callers of DecodeSign1/DecodeMultiSign reasonably expect only CryptographicException from malformed input. Fixes #126036.

Approach: Wrapping ReadInt32() in a ReadInt32ForCrypto() extension method is a clean, DRY solution for the decode paths. The extension is internal, so there's no public API change and no API review needed.

Summary: ⚠️ Needs Human Review. The decode-path fix is correct and well-tested. Two secondary concerns warrant human judgment: (1) in ValidateInsertion, the new CryptographicException escapes with a decode-oriented message where an ArgumentException would be more conventional for the public Add() API, and (2) the MissingCriticalHeaders change is defense-in-depth only (unreachable with current code) and untested.

---

Detailed Findings

✅ Correctness — Decode path fix is sound

The three call sites in CoseMessage.cs (DecodeBucket line 310, MissingCriticalHeaders line 605) correctly use ReadInt32ForCrypto() to catch OverflowException and wrap it as CryptographicException. The CryptographicException propagates through the existing catch filters (which don't match it) and reaches the caller, which is the correct behavior — DecodeSign1 callers expect CryptographicException for malformed input.

The remaining production ReadInt32() call in CoseHeaderValue.GetValueAsInt32() already handles OverflowException independently in its own catch filter (ex is CborContentException or InvalidOperationException or OverflowException), so it does not need this wrapper. Coverage is complete.

✅ InnerException preservation — Good improvement

The change from throw new CryptographicException(e.Message) to throw new CryptographicException(e.Message, e.InnerException) in DecodeBucket (line 326) is a good fix that preserves the root-cause exception (e.g., CborContentException or OverflowException) for all ArgumentException cases, not just the new overflow scenario. This improves diagnostics.

⚠️ ValidateInsertion exception type — CryptographicException escapes public Add() API

In CoseHeaderMap.ValidateInsertion (line 282), ReadInt32ForCrypto() may throw CryptographicException("Error while decoding COSE message..."). This exception is not caught by the surrounding catch filter (ex is CborContentException or InvalidOperationException) and will propagate to callers of the public Add() and indexer-set APIs.

• The message "Error while decoding COSE message" is misleading when the user is adding a header, not decoding.
• The pre-existing behavior was also imperfect (raw OverflowException escaping), so this isn't a regression per se.
• For consistency with the existing pattern (where CborContentException/InvalidOperationException are wrapped as ArgumentException), consider adding or OverflowException to ValidateInsertion's catch filter instead of (or in addition to) using ReadInt32ForCrypto() there:

catch (Exception ex) when (ex is CborContentException or InvalidOperationException or OverflowException)
{
    throw new ArgumentException(SR.Format(SR.CoseHeaderMapArgumentCoseHeaderValueIncorrect, label.LabelName), nameof(value), ex);
}

This would produce ArgumentException (appropriate for a public validation API) while still wrapping the OverflowException as the inner exception. In the decode path (DecodeBucket → Add → ValidateInsertion), the ArgumentException would then be caught by DecodeBucket's catch (ArgumentException e) and lifted to CryptographicException as expected.

Classification: Not merge-blocking (pre-existing issue, arguably improved), but warrants author/reviewer consideration.

💡 Test coverage — MissingCriticalHeaders path is defense-in-depth

• Test 1 (DecodeSign1_ThrowsCryptographicExceptionForOversizedHeaderLabel) exercises the DecodeBucket label-reading path (line 310). ✅
• Test 2 (DecodeSign1_ThrowsCryptographicExceptionForOversizedCritHeaderLabel) exercises the ValidateInsertion crit-header path (line 282), since ValidateInsertion runs before MissingCriticalHeaders during decode. ✅

The ReadInt32ForCrypto() call in MissingCriticalHeaders (line 605) is unreachable in practice because ValidateInsertion would throw first during decoding, preventing the oversized value from being stored. It's good defense-in-depth, but not covered by a test. This is acceptable as-is.

✅ Project file & structure — Correct

The new CborReaderExtensions.cs file is properly listed in the .csproj in alphabetical order. File naming matches the class name.

---

Models contributing to this review: Claude Opus 4.6 (primary), GPT-5.4 (secondary). Gemini was unavailable.

Generated by Code Review for issue #126095 · ◷