Skip to content

der: replace O(n^2) SetOf sort with O(n log n) sort#2345

Merged
tarcieri merged 2 commits into
RustCrypto:masterfrom
arpitjain099:fix/der-sort-complexity-2319
Jun 5, 2026
Merged

der: replace O(n^2) SetOf sort with O(n log n) sort#2345
tarcieri merged 2 commits into
RustCrypto:masterfrom
arpitjain099:fix/der-sort-complexity-2319

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

Fixes #2319

Problem

der_sort() in der/src/asn1/set_of.rs is a hand-rolled insertion sort that runs on every SetOfVec / heapless SetOf decode in DER mode. It is O(n^2), and reverse-sorted input forces n(n-1)/2 comparisons and swaps. A crafted reverse-sorted SET OF (about 18k elements in 246 KB) makes a single SetOfVec::decode_value take roughly 11 seconds, a denial-of-service vector.

This is reachable from any SetOfVec<T> decoded from untrusted DER, including RelativeDistinguishedName in x509-cert (Certificate::from_der) and DigestAlgorithmIdentifiers / CertificateSet / SignerInfos in cms.

Fix

Replace the insertion sort with slice::sort_unstable_by (in-place O(n log n) introsort), capturing the first der_cmp error and returning it after the sort so error bubbling is preserved.

A few correctness points worth calling out:

  • no_std: sort_unstable_by is provided by core, so it compiles under #![no_std] with no alloc. The stable sort_by would require alloc and break the heapless no-alloc build, so unstable is the right choice here. Verified the heapless feature combination compiles and tests pass.
  • Ordering is identical: two elements that compare Equal under DerOrd have byte-identical DER encodings, so an unstable sort cannot change the serialized output. This is cross-checked in a test against a reference copy of the old insertion sort on both randomized and adversarial inputs.
  • Duplicates are preserved: der_sort never deduplicated, and that behavior is unchanged (covered by a test).
  • Non-canonical input is still accepted and sorted at decode time, unchanged.

Before / after

Decoding a reverse-sorted SET OF INTEGER:

  • N=9,000: about 2,818 ms before, 0.35 ms after.
  • N=18,000: about 11,414 ms before, 0.68 ms after (roughly a 16,000x improvement).

Tests

Added der_sort_matches_reference_ordering (old insertion sort vs new sort produce identical ordering on randomized and adversarial inputs), setofvec_decodes_large_reverse_sorted_der, and der_sort_preserves_duplicates. All green across default, alloc, --all-features, and --no-default-features --features heapless. cargo clippy -p der --all-features is clean. x509-cert and cms build against the change.

Out of scope

The issue also notes the SetOfVec (sorts) vs SetOfRef (rejects out-of-order) behavior difference, which a maintainer already commented on; that is a separate behavior question and is not touched here. I also did not add a hard element-count cap, since the algorithmic fix is the clean behavior-preserving change and a cap would reject currently-accepted inputs (a policy call for maintainers).

der_sort was a hand-rolled insertion sort. On reverse-sorted input it
performs n(n-1)/2 comparisons and swaps, so a crafted SET OF turns a
single decode into quadratic-time work. SetOfVec::decode_value runs this
on every DER decode, which makes it a denial-of-service vector on any
SetOfVec parsed from untrusted DER (for example RelativeDistinguishedName
in x509-cert, and DigestAlgorithmIdentifiers / CertificateSet /
SignerInfos in cms). A ~246 KB reverse-sorted SET OF with 18k elements
took roughly 11 seconds to decode.

Switch to slice::sort_unstable_by, an in-place O(n log n) introsort that
is available in core, so it still works on heapless no_std targets where
the stable slice::sort_by (which needs alloc) is not. The first der_cmp
error is captured and returned after the sort, preserving the existing
error-bubbling behavior.

Sorting unstably does not change the result: two elements that compare
Equal under DerOrd have identical DER encodings, so their relative order
does not affect the serialized output. Duplicates are still preserved
(no deduplication), matching the decoder behavior after #2272.

Adds regression tests that cross-check the new ordering against the
original insertion sort on randomized and adversarial inputs, assert the
DER SET OF ordering invariant, verify duplicate preservation, and decode
a large reverse-sorted SET OF end to end.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
CI under -D warnings flagged four lints in the tests added by this
branch, plus a rustfmt diff. All are in test code; the der_sort fix
itself is unchanged.

- cast_possible_truncation (u64 -> u16 in the LCG helper): mask the
  low 16 bits explicitly instead of a lossy as-cast, so the truncation
  is intentional and obvious.
- cast_possible_truncation (usize -> u8 in the DER length encoder):
  use u8::try_from with an expect documenting the in-range invariant
  (short-form length < 0x80; significant length-byte count <= 8).
- doc_markdown: add backticks around DoS in a doc comment.
- rustfmt: wrap the long LCG expression onto multiple lines.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@tarcieri tarcieri merged commit 787398b into RustCrypto:master Jun 5, 2026
116 checks passed
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.

SetOfVec::decode_value uses O(N²) insertion sort — quadratic DoS on crafted SET OF input

2 participants