fix(security): bound panics on hostile authenticator inputs

libwebauthn/src/ops/u2f.rs

@@ -89,7 +89,18 @@ impl UpgradableResponse<MakeCredentialResponse, MakeCredentialRequest> for Regis
             y: y.into(),
         });
         let cose_encoded_public_key = cbor::to_vec(&cose_public_key)?;
-        assert!(cose_encoded_public_key.len() == 77);
+        // Canonical CBOR encoding of the COSE P-256 key is 77 bytes for the
+        // fields we set; return a typed error if a future encoder change
+        // produces a different length rather than `assert!`-panicking.
+        if cose_encoded_public_key.len() != 77 {
+            error!(
+                len = cose_encoded_public_key.len(),
+                "COSE-encoded P-256 public key is not 77 bytes"
+            );
+            return Err(Error::Platform(PlatformError::CryptoError(
+                "unexpected COSE-encoded public key length".into(),
+            )));
+        }
 
         // Let attestedCredData be a byte string with following structure:
         //

libwebauthn/src/pin.rs

@@ -175,11 +175,23 @@ where
             );
             return Err(Error::Ctap(CtapError::Other));
         };
-        let encoded_point = EncodedPoint::from_affine_coordinates(
-            peer_public_key.x.as_bytes().into(),
-            peer_public_key.y.as_bytes().into(),
-            false,
-        );
+        // x and y must be exactly 32 bytes (P-256 field size). `cosey` accepts
+        // any length up to 32; validate before converting to `&FieldBytes`.
+        let x: &[u8; 32] = peer_public_key.x.as_bytes().try_into().map_err(|_| {
+            error!(
+                x_len = peer_public_key.x.as_bytes().len(),
+                "Peer public key x coordinate is not 32 bytes"
+            );
+            Error::Ctap(CtapError::Other)
+        })?;
+        let y: &[u8; 32] = peer_public_key.y.as_bytes().try_into().map_err(|_| {
+            error!(
+                y_len = peer_public_key.y.as_bytes().len(),
+                "Peer public key y coordinate is not 32 bytes"
+            );
+            Error::Ctap(CtapError::Other)
+        })?;
+        let encoded_point = EncodedPoint::from_affine_coordinates(x.into(), y.into(), false);
         let Some(peer_public_key) = P256PublicKey::from_encoded_point(&encoded_point).into() else {
             error!("Failed to parse public key.");
             return Err(Error::Ctap(CtapError::Other));
@@ -350,7 +362,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
 
     fn encrypt(&self, key: &[u8], plaintext: &[u8]) -> Result<Vec<u8>, Error> {
         // Discard the first 32 bytes of key. (This selects the AES-key portion of the shared secret.)
-        let key = &key[32..];
+        let key = key.get(32..).ok_or_else(|| {
+            error!(
+                key_len = key.len(),
+                "key shorter than 32 bytes; cannot select AES-key portion"
+            );
+            Error::Ctap(CtapError::Other)
+        })?;
 
         // Let iv be a 16-byte, random bytestring.
         let iv: [u8; 16] = thread_rng().gen();
@@ -371,7 +389,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
 
     fn decrypt(&self, key: &[u8], ciphertext: &[u8]) -> Result<Vec<u8>, Error> {
         // Discard the first 32 bytes of key. (This selects the AES-key portion of the shared secret.)
-        let key = &key[32..];
+        let key = key.get(32..).ok_or_else(|| {
+            error!(
+                key_len = key.len(),
+                "key shorter than 32 bytes; cannot select AES-key portion"
+            );
+            Error::Ctap(CtapError::Other)
+        })?;
 
         // If demPlaintext is less than 16 bytes in length, return an error
         if ciphertext.len() < 16 {
@@ -397,7 +421,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
     fn authenticate(&self, key: &[u8], message: &[u8]) -> Result<Vec<u8>, Error> {
         // If key is longer than 32 bytes, discard the excess. (This selects the HMAC-key portion of the shared secret.
         // When key is the pinUvAuthToken, it is exactly 32 bytes long and thus this step has no effect.)
-        let key = &key[..32];
+        let key = key.get(..32).ok_or_else(|| {
+            error!(
+                key_len = key.len(),
+                "key shorter than 32 bytes; cannot select HMAC-key portion"
+            );
+            Error::Ctap(CtapError::Other)
+        })?;
 
         // Return the result of computing HMAC-SHA-256 on key and message.
         hmac_sha256(key, message)
@@ -578,3 +608,82 @@ where
             .await
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use cosey::{Bytes, EcdhEsHkdf256PublicKey};
+
+    fn make_peer_key(x: &[u8], y: &[u8]) -> cose::PublicKey {
+        cose::PublicKey::EcdhEsHkdf256Key(EcdhEsHkdf256PublicKey {
+            x: Bytes::from_slice(x).unwrap(),
+            y: Bytes::from_slice(y).unwrap(),
+        })
+    }
+
+    #[test]
+    fn ecdh_rejects_short_x() {
+        let proto = PinUvAuthProtocolOne::new();
+        let x = vec![0x01u8; 31];
+        let y = vec![0x02u8; 32];
+        let key = make_peer_key(&x, &y);
+
+        let result = PinUvAuthProtocol::encapsulate(&proto, &key);
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn ecdh_rejects_empty_x() {
+        let proto = PinUvAuthProtocolOne::new();
+        let x: Vec<u8> = Vec::new();
+        let y = vec![0x02u8; 32];
+        let key = make_peer_key(&x, &y);
+
+        let result = PinUvAuthProtocol::encapsulate(&proto, &key);
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn ecdh_rejects_short_y() {
+        let proto = PinUvAuthProtocolTwo::new();
+        let x = vec![0x01u8; 32];
+        let y = vec![0x02u8; 16];
+        let key = make_peer_key(&x, &y);
+
+        let result = PinUvAuthProtocol::encapsulate(&proto, &key);
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn proto_two_authenticate_rejects_empty_key() {
+        let proto = PinUvAuthProtocolTwo::new();
+        let result = proto.authenticate(&[], b"clientDataHash");
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn proto_two_authenticate_rejects_short_key() {
+        let proto = PinUvAuthProtocolTwo::new();
+        let short_key = [0u8; 16];
+        let result = proto.authenticate(&short_key, b"hello");
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn proto_two_encrypt_rejects_short_key() {
+        let proto = PinUvAuthProtocolTwo::new();
+        let short_key = [0u8; 16];
+        let plaintext = [0u8; 16];
+        let result = proto.encrypt(&short_key, &plaintext);
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+
+    #[test]
+    fn proto_two_decrypt_rejects_short_key() {
+        let proto = PinUvAuthProtocolTwo::new();
+        let short_key = [0u8; 16];
+        let ct = [0u8; 32];
+        let result = proto.decrypt(&short_key, &ct);
+        assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
+    }
+}

libwebauthn/src/proto/ctap2/model/get_info.rs

@@ -246,7 +246,16 @@ impl Ctap2GetInfoResponse {
 
             // If we do have a PIN, check if we need to use legacy getPinToken or new getPinUvAuthToken..-command
             if self.option_enabled("pinUvAuthToken") {
-                assert!(self.option_enabled("clientPin"));
+                // `pinUvAuthToken` and `clientPin` are independent options
+                // (CTAP 2.2 §6.4). Tolerate the combination without `clientPin`
+                // and fall back to a shared-secret-only flow.
+                if !self.option_enabled("clientPin") {
+                    debug!(
+                        "Device advertises pinUvAuthToken without clientPin; \
+                         falling back to OnlyForSharedSecret"
+                    );
+                    return Some(Ctap2UserVerificationOperation::OnlyForSharedSecret);
+                }
                 debug!("getPinUvAuthTokenUsingPinWithPermissions");
                 Some(Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingPinWithPermissions)
             } else if self.option_enabled("clientPin") {
@@ -574,4 +583,17 @@ mod test {
             Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
         );
     }
+
+    #[test]
+    fn device_pin_uv_auth_token_without_client_pin_does_not_panic() {
+        let info = create_info(&[("pinUvAuthToken", true)]);
+        assert_eq!(
+            info.uv_operation(false),
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
+        );
+        assert_eq!(
+            info.uv_operation(true),
+            Some(Ctap2UserVerificationOperation::OnlyForSharedSecret)
+        );
+    }
 }

libwebauthn/src/transport/cable/tunnel.rs

@@ -523,6 +523,31 @@ async fn connection_recv_binary_frame(message: Message) -> Result<Option<Vec<u8>
     }
 }
 
+/// Strip the trailing padding-length byte and `padding_len` bytes of padding
+/// from a decrypted Noise transport frame, returning `InvalidFraming` on an
+/// empty plaintext or a declared padding length that exceeds the frame.
+fn strip_frame_padding(mut decrypted_frame: Vec<u8>) -> Result<Vec<u8>, Error> {
+    let padding_len = match decrypted_frame.last() {
+        Some(&b) => b as usize,
+        None => {
+            error!("Decrypted frame is empty; cannot read padding length");
+            return Err(Error::Transport(TransportError::InvalidFraming));
+        }
+    };
+    let new_len = decrypted_frame
+        .len()
+        .checked_sub(padding_len + 1)
+        .ok_or_else(|| {
+            error!(
+                frame_len = decrypted_frame.len(),
+                padding_len, "Padding length exceeds frame length"
+            );
+            Error::Transport(TransportError::InvalidFraming)
+        })?;
+    decrypted_frame.truncate(new_len);
+    Ok(decrypted_frame)
+}
+
 async fn decrypt_frame(
     encrypted_frame: Vec<u8>,
     noise_state: &mut TunnelNoiseState,
@@ -543,8 +568,7 @@ async fn decrypt_frame(
         }
     }
 
-    let padding_len = decrypted_frame[decrypted_frame.len() - 1] as usize;
-    decrypted_frame.truncate(decrypted_frame.len() - (padding_len + 1));
+    let decrypted_frame = strip_frame_padding(decrypted_frame)?;
     trace!(
         ?decrypted_frame,
         decrypted_frame_len = decrypted_frame.len(),
@@ -795,4 +819,32 @@ mod tests {
     }
 
     // TODO: test the non-known case
+
+    #[test]
+    fn strip_frame_padding_rejects_empty() {
+        let result = strip_frame_padding(Vec::new());
+        assert!(matches!(
+            result,
+            Err(Error::Transport(TransportError::InvalidFraming))
+        ));
+    }
+
+    #[test]
+    fn strip_frame_padding_rejects_overlong_padding() {
+        // Length 1 + declared padding of 5 -> would require subtracting 6 from 1.
+        let frame = vec![0x05u8];
+        let result = strip_frame_padding(frame);
+        assert!(matches!(
+            result,
+            Err(Error::Transport(TransportError::InvalidFraming))
+        ));
+    }
+
+    #[test]
+    fn strip_frame_padding_strips_normal_padding() {
+        // 4 bytes of payload, 3 bytes of zero padding, then padding-length 3.
+        let frame = vec![0xAA, 0xBB, 0xCC, 0xDD, 0x00, 0x00, 0x00, 0x03];
+        let stripped = strip_frame_padding(frame).unwrap();
+        assert_eq!(stripped, vec![0xAA, 0xBB, 0xCC, 0xDD]);
+    }
 }

libwebauthn/src/webauthn/pin_uv_auth_token.rs

@@ -359,6 +359,22 @@ where
                 let uv_auth_token =
                     uv_proto.decrypt(&shared_secret, &encrypted_pin_uv_auth_token)?;
 
+                // pinUvAuthToken is 16 bytes for PUAP1 and 32 bytes for PUAP2.
+                // Reject a shorter token before it is used as a key downstream.
+                let min_token_len = match uv_proto.version() {
+                    Ctap2PinUvAuthProtocol::One => 16,
+                    Ctap2PinUvAuthProtocol::Two => 32,
+                };
+                if uv_auth_token.len() < min_token_len {
+                    error!(
+                        protocol = ?uv_proto.version(),
+                        token_len = uv_auth_token.len(),
+                        min_expected = min_token_len,
+                        "Decrypted pinUvAuthToken is shorter than required"
+                    );
+                    return Err(Error::Ctap(CtapError::Other));
+                }
+
                 let token_identifier = Ctap2AuthTokenPermission::new(
                     uv_proto.version(),
                     ctap2_request.permissions(),
@@ -838,7 +854,7 @@ mod test {
         .unwrap();
         // We do here what the device would need to do, i.e. generate a new random
         // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
-        let token = [5; 32];
+        let token = [5; 16];
         let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
         let pin_resp = CborResponse::new_success_from_slice(
             to_vec(&Ctap2ClientPinResponse {
@@ -1182,7 +1198,7 @@ mod test {
                 .unwrap();
             // We do here what the device would need to do, i.e. generate a new random
             // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
-            let token = [5; 32];
+            let token = [5; 16];
             let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
             let pin_resp = CborResponse::new_success_from_slice(
                 to_vec(&Ctap2ClientPinResponse {
@@ -1322,7 +1338,7 @@ mod test {
                 .unwrap();
             // We do here what the device would need to do, i.e. generate a new random
             // pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
-            let token = [5; 32];
+            let token = [5; 16];
             let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
             let pin_resp = CborResponse::new_success_from_slice(
                 to_vec(&Ctap2ClientPinResponse {