Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionCertificate length was not properly checked when added to a certificate store. In practice only trusted data was processed.
ComponentSecurity/NSS
Bug ClassLogic Error
Tracker1940804
Fix commit3f830053b78c (firefox) +22/-18
CISA KEVNot listed
CreditedTheemathas
Disclosed2025-02-04

Files Changed

  • security/manager/ssl/cert_storage/src/lib.rs
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…