[Audit-Low] Slic Initialize parameter dictionary has no cardinality cap

[pepone]

AI-generated audit finding — this issue was opened from an automated security/correctness audit. It has not been triaged by a human yet; verify the reasoning, reproducibility, and severity before acting on it.

Low: Slic Initialize parameter dictionary has no cardinality cap

Affected code:

• src/IceRpc/Transports/Slic/Internal/SlicConnection.cs:904-962 — DecodeParameters
• src/IceRpc/Transports/Slic/Internal/SlicConnection.cs:45-48 — MaxControlFrameBodySize = 16,383

Description:

Initialize / InitializeAck frame bodies are capped at 16,383 bytes by MaxControlFrameBodySize (added in #4516), and the individual parameter values that we care about are range-checked (MaxStreamFrameSize, InitialStreamWindowSize ≥ 1 KB; IdleTimeout ≠ 0). However:

1. There is no explicit cap on the number of entries in the parameter dictionary. Within the 16 KB body, a peer can pack a large number of small unknown parameters — each is decoded into the IDictionary<ParameterKey, IList<byte>> and then ignored by the switch (the // Ignore unsupported parameter branch at line 960 has no default: rejection).
2. Duplicate parameter keys are not explicitly rejected by DecodeParameters; handling depends on how the underlying Slice dictionary decoder reacts to duplicates, and that contract is not asserted at this call site.

Impact: Low — bounded by the 16 KB frame cap. This is defense-in-depth hardening, not an exploitable DoS.

Recommendation:

• Add an explicit cap on the number of parameters accepted in DecodeParameters (e.g. 32) and throw InvalidDataException beyond it.
• Either reject unknown parameter keys outright, or document that unknown keys are silently ignored and bound their count as above.
• Assert (and test) behavior on duplicate keys — either reject or define a deterministic resolution.

Related: #4409, #4410 (other peer-parameter range checks), #4516 (body-size cap), #3317 (umbrella).

---

Source report: Slic security review against HTTP/2 CVE classes (CVE-2019-9512..9518, CVE-2023-44487). Companion findings with higher sensitivity are tracked privately in icerpc/icerpc-csharp-audit.

[pepone]

Closing as not planned.

The audit identifies three potential gaps; on closer look only one is real, and it's already mitigated by the SliceDecoder's general collection-allocation budget:

1. Duplicate parameter keys — already rejected. SliceDecoderExtensions.DecodeDictionary (used by the generated InitializeBody/InitializeAckBody constructors) catches the ArgumentException from Dictionary.Add and rethrows as InvalidDataException("Received dictionary with duplicate key …"). This matches QUIC's TRANSPORT_PARAMETER_ERROR semantics for duplicates.

2. Unknown parameter keys — intentionally ignored. Both HTTP/2 (RFC 9113 §6.5.2) and QUIC (RFC 9000 §7.4.2) mandate ignoring unknown identifiers for forward compatibility; rejecting them would prevent rolling out new parameters without breaking older peers. We deliberately follow the same convention.

3. Cardinality cap — bounded by SliceDecoder's collection-allocation budget plus the 16 KB control-frame body cap. The decoder built by DecodeSliceBuffer defaults maxCollectionAllocation to 8 × buffer.Length (SliceDecoder.cs:55-62). For a 16,383-byte Initialize body that's ≈ 131 KB. DecodeDictionary consumes that budget at IncreaseCollectionAllocation(count, sizeof(TKey) + sizeof(TValue)) before it adds any entries, so a peer that declares an inflated entry count fails with InvalidDataException. A peer that honestly fills the body with ~8,000 minimum-size entries causes a single ~500 KB allocation spike that's GC'd as soon as DecodeParameters returns, since the dictionary is method-local.

So the worst case is a one-shot ~500 KB amplification per malicious Initialize, bounded by connection accept rate and TLS handshake cost. The audit itself rates this Low / "defense-in-depth hardening, not an exploitable DoS," and the SliceDecoder's collection-allocation budget is the architectural mitigation we're relying on. Adding a parameter-count cap on top would be cosmetic — it would reject the shape after the dictionary is already allocated, with no real reduction in attacker-induced allocation.

If we revisit, the right place is the SliceDecoder budget (already in place) or a custom dictionary factory at the decode site. Closing without code change.