Skip to content

fix: full depth tracking across Codable container hierarchy#124

Open
hamchapman wants to merge 2 commits intohc/fix-unimplemented-codable-superencoder-methodsfrom
hc/fix-maximum-depth-not-enforced-dos-vulnerability
Open

fix: full depth tracking across Codable container hierarchy#124
hamchapman wants to merge 2 commits intohc/fix-unimplemented-codable-superencoder-methodsfrom
hc/fix-maximum-depth-not-enforced-dos-vulnerability

Conversation

@hamchapman
Copy link
Copy Markdown
Collaborator

This change includes 2 main changes.

1. Enforcing maximumDepth in CodableCBORDecoder

Previously, the maximumDepth option was defined but not actually used in
CodableCBORDecoder, creating a DoS vulnerability where deeply nested
structures could cause stack overflow.

Problem:

  1. CodableCBORDecoder had a maximumDepth property but it wasn't included
    in the options computed property, so it defaulted to .max
  2. SingleValueDecodingContainer called CBOR.decode() without passing options
  3. KeyedDecodingContainer and UnkeyedDecodingContainer created CBORDecoder
    instances without passing options
  4. This meant depth checking was never enforced for Codable decoding

Fix:

  • Add maximumDepth property to CodableCBORDecoder (public API)
  • Include maximumDepth in options computed property
  • Update setOptions() to set maximumDepth
  • Pass options to all CBOR.decode() calls in SingleValueDecodingContainer
  • Pass options to CBORDecoder() calls in KeyedDecodingContainer
  • Pass options to CBORDecoder() calls in UnkeyedDecodingContainer

2. Implement full depth tracking across Codable container hierarchy

Changes:

  • Added currentDepth field to _CBORDecoder and all container classes
    (KeyedContainer, UnkeyedContainer, SingleValueContainer)
  • Thread currentDepth through all container creation and nested decoding
  • Check depth before creating keyed/unkeyed containers in _CBORDecoder
  • Increment depth when creating nested decoders in decode() methods
  • Pass incremented depth through superDecoder() methods

Previously, each CBOR.decode() call would reset currentDepth to 0, allowing
deeply nested structures to bypass the depth limit. For example, a 10-level
nested array with maximumDepth=5 would incorrectly succeed. Now depth is
properly tracked across the entire container hierarchy.

This change includes 2 main changes.

Previously, the `maximumDepth` option was defined but not actually used in
`CodableCBORDecoder`, creating a DoS vulnerability where deeply nested
structures could cause stack overflow.

**Problem:**

1. `CodableCBORDecoder` had a `maximumDepth` property but it wasn't included
   in the options computed property, so it defaulted to `.max`
2. `SingleValueDecodingContainer` called `CBOR.decode()` without passing options
3. `KeyedDecodingContainer` and `UnkeyedDecodingContainer` created `CBORDecoder`
   instances without passing options
4. This meant depth checking was never enforced for Codable decoding

**Fix:**

- Add `maximumDepth` property to `CodableCBORDecoder` (public API)
- Include `maximumDepth` in options computed property
- Update `setOptions()` to set `maximumDepth`
- Pass options to all `CBOR.decode()` calls in `SingleValueDecodingContainer`
- Pass options to `CBORDecoder()` calls in `KeyedDecodingContainer`
- Pass options to `CBORDecoder()` calls in `UnkeyedDecodingContainer`

Changes:
- Added `currentDepth` field to `_CBORDecoder` and all container classes
  (`KeyedContainer`, `UnkeyedContainer`, `SingleValueContainer`)
- Thread `currentDepth` through all container creation and nested decoding
- Check depth before creating keyed/unkeyed containers in `_CBORDecoder`
- Increment depth when creating nested decoders in `decode()` methods
- Pass incremented depth through `superDecoder()` methods

Previously, each `CBOR.decode()` call would reset `currentDepth` to 0, allowing
deeply nested structures to bypass the depth limit. For example, a 10-level
nested array with `maximumDepth=5` would incorrectly succeed. Now depth is
properly tracked across the entire container hierarchy.
@hamchapman hamchapman self-assigned this Feb 2, 2026
This fixes two related issues introduced when CBOR tag processing was enabled:

1. Date decoding: Tag 1 timestamps are converted to CBOR.date values, but
   SingleValueDecodingContainer didn't handle the .date case when decoding
   Double, causing type mismatch errors. Fixed by returning
   date.timeIntervalSinceReferenceDate.

2. Byte string as array: Foundation types (CharacterSet, Data) encode
   internal data as CBOR byte strings but expect to decode via
   unkeyedContainer(). The decoder now:
   - Accepts byte strings (0x40-0x5f) in ensureArray()
   - Passes an isByteString flag to UnkeyedContainer constructor
   - Only sets this flag for top-level byte string containers, not nested
     containers, preventing false positives when nested arrays happen to
     have format bytes in the byte string range
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant