fix(x509): use checked length arithmetic in CertificateBundle::try_from

[schenkty]

Summary

CertificateBundle::try_from(&[u8]) parses concatenated DER certificates using hand-rolled length arithmetic with unchecked additions:

let total_len = header_len + cert_len;
if offset + total_len <= data.len() {
    let cert_data = &data[offset..offset + total_len];

parse_der_length folds an attacker-chosen long-form DER length (up to 8 bytes) into a usize with no width cap, so cert_len can be ≈ usize::MAX. With the header 30 88 FF FF FF FF FF FF FF F5, cert_len = 0xFFFFFFFFFFFFFFF5 and header_len = 10, so total_len = usize::MAX (no overflow at that add). Once offset has advanced past one valid certificate (offset ≥ 1), offset + total_len wraps to offset - 1, which satisfies the <= data.len() guard — and the following slice &data[offset..offset-1] has start > end and panics. Under the release profile's panic = "abort" this aborts the whole process/wasm instance.

Reachable from the WASM export X509Certificate.parseChain (keetanetwork-client-wasm), which hex-decodes a caller-supplied string straight into CertificateBundle::try_from.

Reproduced arithmetic:

cert_len=0xfffffffffffffff5, header_len=10  ->  total_len=usize::MAX
offset(100)+total_len wraps to 99  <=  data.len()(120)  =>  guard passes
&data[100..99]  => 'slice index starts at 100 but ends at 99'  => abort

(No memory-safety corruption — Rust's bounds check catches the out-of-range slice before any read; the impact is a DoS abort.)

Fix

• checked_add for header_len + cert_len and offset + total_len; break on overflow.
• Defense-in-depth in parse_der_length: reject long-form lengths wider than size_of::<usize>() (which would otherwise silently shift out high bits).

Tests

cargo test -p keetanetwork-x509 — all 71 pass, including:

• new test_bundle_try_from_rejects_overflow_length_without_panic — a valid cert followed by the malicious length header parses without panicking and retains only the leading valid cert.
• existing test_parse_der_length still passes.

Severity: medium (DoS / process abort in a public decode path).

Draft — audit fix; do not merge without maintainer review.

  

[larseidsvoll]

Overflow→slice panic path fixed with checked arithmetic and width cap. REQUEST_CHANGES: Lint red on this head — reformat (long if in utils.rs) and re-run. Draft + Ty lock — no merge.

[larseidsvoll]

Prior REQUEST_CHANGES cleared — Lint green and checked-length fix intact. Draft + Ty lock — no merge.

[larseidsvoll]

Verdict: APPROVE (re-stamp after #45)

Tip moved for main merge. Diff unchanged: checked DER length arithmetic + width cap + overflow regression test.

Security Audit green. Lint green; Tests still running.

Review only — do not bot-merge. Humans merge. No @ humans.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[larseidsvoll]

No security issues found.

Tip e16ab92ac33733c39294b3df143d8251d4ac702a: checked CertificateBundle length arithmetic (checked_add) and parse_der_length usize-width guard.