Skip to content

der: return an error instead of panicking on truncated binary REAL values - #2483

Open
choksi2212 wants to merge 1 commit into
RustCrypto:masterfrom
choksi2212:fix/der-real-truncated-exponent
Open

choksi2212 wants to merge 1 commit into
RustCrypto:masterfrom
choksi2212:fix/der-real-truncated-exponent

Conversation

@choksi2212

Copy link
Copy Markdown

Summary

The binary-encoding branch of the REAL (f64) decoder (X.690 section 8.5.7, der/src/asn1/real.rs) reads the exponent octets at fixed offsets in the value content without confirming the content is long enough to hold them: the one-octet exponent form indexes bytes[1], and the two-octet exponent form indexes bytes[1] and bytes[2] (GHSA-27m6-7r55-vwr2, #2482).

A REAL whose value content is the single octet 0x80 therefore reaches bytes[1] on a length-1 slice and panics instead of returning Err:

thread '...' panicked at der/src/asn1/real.rs:45:60:
index out of bounds: the len is 1 but the index is 1

Any caller decoding f64 values (or structs containing them) from untrusted DER can be taken down by a two-byte input (09 01 80).

Changes

  • Confirm the content length before reading the exponent octets in both exponent forms (>= 2 for the one-octet form, >= 3 for the two-octet form), returning the same Tag::Real value error the function already uses for other malformed binary REALs. No public API or behavior change for well-formed values.
  • Regression test pinning all three truncation shapes (0x80 short form, 0x81 short form with no exponent octets, 0x81 00 two-octet form missing the second exponent octet).

Evidence

  • On current master the new test fails with the exact panic above (index out of bounds: the len is 1 but the index is 1); with this change all three inputs return Err.
  • cargo test -p der --features real (lib): 92 passed, 0 failed.
  • cargo test -p der --no-default-features --features real: green.
  • cargo test -p der --features arbitrary,ber,bytes,derive,oid,pem,real,std: green except the pre-existing tests/pem.rs::to_pem CRLF expectation, which fails identically on pristine master on a Windows checkout (PEM line endings), unrelated to this change.
  • cargo build -p der --no-default-features (no_std): builds clean.
  • RUSTFLAGS=-Dwarnings cargo clippy -p der --all-features --tests: clean. cargo fmt -p der -- --check: clean.

Notes

This is the minimal non-breaking point fix for the published advisory; whether the REAL feature is removed outright in the next breaking release is a separate call (#2482).

…lues

The binary-encoding branch of the REAL (f64) decoder (X.690 section
8.5.7) reads the exponent octets at fixed offsets in the value content
without checking that the content is long enough to hold them: the
one-octet exponent form indexes bytes[1] and the two-octet exponent
form indexes bytes[1] and bytes[2].

A REAL value whose content is the single octet 0x80 therefore reaches
bytes[1] on a length-1 slice and panics with 'index out of bounds'
instead of returning Err, taking down any caller that decodes f64
values (or structs containing them) from untrusted input.

Confirm the content length before reading the exponent octets, in both
exponent forms, and return a Real value error otherwise.

Fixes RustCrypto#2482

Signed-off-by: choksi2212 <choksi2212@users.noreply.github.com>

This branch has not been deployed

No deployments
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