Firefox · Security/NSS
CVE-2025-1014
Logic Error in Security/NSS
Overview
Low
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `cert_storage`
- Firefox’s Rust component (
security/manager/ssl/cert_storage/src/lib.rs) that persists certificates, trust records, and revocation data in anLMDB-backed key-value store. - `u16::MAX` vs `u16::max`
u16::MAXis the constant65535, whereasu16::maxis a method whose name-as-value coerces to a function item and, when cast withas usize, yields the function’s address rather than65535.- Length-prefixed serialization
Certrecords are written with au16NetworkEndianlength prefix before each variable-length field (der,subject), so any field longer than65535bytes cannot be represented in the prefix.- `try_into` / `into`
- Rust conversions where
intois infallible (widening) andtry_intoreturns aResultthat surfaces an error on a lossy narrowing instead of silently truncating.
Root Cause Analysis
The bounds checks in Cert::new and the serializer guarded certificate and subject lengths with if der.len() > u16::max as usize, which was meant to compare against 65535 but instead compared against the address of the u16::max function coerced to usize. Because that address is an arbitrary large value unrelated to the intended limit, the guard did not reliably reject oversized inputs, violating the invariant that a field’s length must fit in the u16 length prefix used during serialization. In the serializer, the subsequent self.der.len() as u16 cast would then silently truncate a length above 65535, producing a length prefix inconsistent with the appended bytes and corrupting the stored/parsed record.
The fix replaces the constant with u16::MAX.into() for the explicit checks and, more robustly, replaces the lossy as u16 casts with try_into().map_err(...) so an out-of-range length now returns a SecurityStateError instead of truncating. A parallel change in the store-read helper swaps a range-guarded match for try_into so an out-of-range I64 value produces an error rather than a silent narrowing to i16.
Key insight
The single core mistake was writing
u16::max (a method item whose cast address is meaningless) where u16::MAX (the 65535 constant) was intended, so the length guard never enforced the real limit. The fix both corrects the constant and moves to try_into, making any lossy narrowing a hard error rather than silent truncation.Attack Path
- Supply an oversized field
A record with a
derorsubjectblob longer than65535bytes is passed towardCert::new/ serialization. - Broken guard is bypassed
The
der.len() > u16::max as usizecheck compares against a function address rather than65535, so the oversized length is not rejected. - Truncating cast on write
The serializer’s
self.der.len() as u16truncates the true length modulo65536, writing a length prefix that disagrees with the appended byte count. - Corrupted record on read
On deserialization the truncated prefix causes the parser to read the wrong number of bytes, yielding a malformed or misaligned
Certin the certificate store.
Impact Assessment
An attacker who could feed an oversized certificate or subject into
cert_storage could cause silent length truncation and a corrupted store entry within the browser’s parent (privileged) certificate-management context. In practice the inputs to this path are trusted data (Mozilla-controlled revocation/trust material), so the preconditions for malicious exploitation are not met, keeping the real-world severity low despite the logic error.Files Changed
security/manager/ssl/cert_storage/src/lib.rs
Audit Directions
- Constant-vs-method confusionGrep for
::max as/::min asand similar casts of associated functions where theMAX/MINassociated constants were intended. - Lossy `as` casts on lengthsReview every
len() as u16/as u8/as i16used to build length prefixes or store sizes, and prefertry_intoso truncation becomes an explicit error. - Guarded narrowing on store readsAudit all
Value::I64/Value::Blobreads that narrow into smaller integer types, ensuring out-of-range values are rejected viatry_intorather than range-guarded matches that fall through to silent casts.
Patch
diff --git a/security/manager/ssl/cert_storage/src/lib.rs b/security/manager/ssl/cert_storage/src/lib.rs
index 83e9beb09db..ad79e6ce135 100644
--- a/security/manager/ssl/cert_storage/src/lib.rs
+++ b/security/manager/ssl/cert_storage/src/lib.rs
@@ -495,10 +495,10 @@ impl SecurityState {
};
let reader = env_and_store.env.read()?;
match env_and_store.store.get(&reader, key) {
- Ok(Some(Value::I64(i)))
- if i <= (std::i16::MAX as i64) && i >= (std::i16::MIN as i64) =>
- {
- Ok(Some(i as i16))
+ Ok(Some(Value::I64(i))) => {
+ Ok(Some(i.try_into().map_err(|_| {
+ SecurityStateError::from("Stored value out of range for i16")
+ })?))
}
Ok(None) => Ok(None),
Ok(_) => Err(SecurityStateError::from(
@@ -1034,10 +1034,10 @@ struct Cert<'a> {
impl<'a> Cert<'a> {
fn new(der: &'a [u8], subject: &'a [u8], trust: i16) -> Result<Cert<'a>, SecurityStateError> {
- if der.len() > u16::max as usize {
+ if der.len() > u16::MAX.into() {
return Err(SecurityStateError::from("certificate is too long"));
}
- if subject.len() > u16::max as usize {
+ if subject.len() > u16::MAX.into() {
return Err(SecurityStateError::from("subject is too long"));
}
Ok(Cert {
@@ -1061,7 +1061,7 @@ impl<'a> Cert<'a> {
return Err(SecurityStateError::from("invalid Cert: no der len?"));
}
let (mut der_len, rest) = rest.split_at(size_of::<u16>());
- let der_len = der_len.read_u16::<NetworkEndian>()? as usize;
+ let der_len = der_len.read_u16::<NetworkEndian>()?.into();
if rest.len() < der_len {
return Err(SecurityStateError::from("invalid Cert: no der?"));
}
@@ -1071,7 +1071,7 @@ impl<'a> Cert<'a> {
return Err(SecurityStateError::from("invalid Cert: no subject len?"));
}
let (mut subject_len, rest) = rest.split_at(size_of::<u16>());
- let subject_len = subject_len.read_u16::<NetworkEndian>()? as usize;
+ let subject_len = subject_len.read_u16::<NetworkEndian>()?.into();
if rest.len() < subject_len {
return Err(SecurityStateError::from("invalid Cert: no subject?"));
}
@@ -1102,15 +1102,19 @@ impl<'a> Cert<'a> {
+ size_of::<i16>(),
);
bytes.write_u8(CERT_SERIALIZATION_VERSION_1)?;
- if self.der.len() > u16::max as usize {
- return Err(SecurityStateError::from("certificate is too long"));
- }
- bytes.write_u16::<NetworkEndian>(self.der.len() as u16)?;
+ bytes.write_u16::<NetworkEndian>(
+ self.der
+ .len()
+ .try_into()
+ .map_err(|_| SecurityStateError::from("certificate is too long"))?,
+ )?;
bytes.extend_from_slice(&self.der);
- if self.subject.len() > u16::max as usize {
- return Err(SecurityStateError::from("subject is too long"));
- }
- bytes.write_u16::<NetworkEndian>(self.subject.len() as u16)?;
+ bytes.write_u16::<NetworkEndian>(
+ self.subject
+ .len()
+ .try_into()
+ .map_err(|_| SecurityStateError::from("subject is too long"))?,
+ )?;
bytes.extend_from_slice(&self.subject);
bytes.write_i16::<NetworkEndian>(self.trust)?;
Ok(bytes)
@@ -1324,7 +1328,7 @@ fn load_crlite_stash_from_reader_into_map(
let issuer_spki_hash_len = reader.read_u8().map_err(|e| {
SecurityStateError::from(format!("error reading stash issuer_spki_hash_len: {}", e))
})?;
- let mut issuer_spki_hash = vec![0; issuer_spki_hash_len as usize];
+ let mut issuer_spki_hash = vec![0; issuer_spki_hash_len.into()];
reader.read_exact(&mut issuer_spki_hash).map_err(|e| {
SecurityStateError::from(format!("error reading stash issuer_spki_hash: {}", e))
})?;
@@ -1333,7 +1337,7 @@ fn load_crlite_stash_from_reader_into_map(
let serial_len = reader.read_u8().map_err(|e| {
SecurityStateError::from(format!("error reading stash serial_len: {}", e))
})?;
- let mut serial = vec![0; serial_len as usize];
+ let mut serial = vec![0; serial_len.into()];
reader.read_exact(&mut serial).map_err(|e| {
SecurityStateError::from(format!("error reading stash serial: {}", e))
})?;
Loading diff…
References
On This Page