Skip to content

Commit 99fa265

Browse files
fix(pin): bounds-checked slicing in PIN/UV protocol 2 primitives
`PinUvAuthProtocolTwo::{encrypt, decrypt}` use `&key[32..]` to discard the HMAC-key portion of the shared secret, and `authenticate` uses `&key[..32]`. Both panic with an out-of-bounds slice index if the key is shorter than 32 bytes. This is reachable from device-controlled data: in `user_verification`, `uv_proto.decrypt(&shared_secret, &encrypted_pin_uv_auth_token)?` yields a pinUvAuthToken of `encrypted_pin_uv_auth_token.len() - 16` bytes; a malicious authenticator returning a 16-byte IV-only ciphertext decrypts to an empty token, which then panics `PinUvAuthProtocolTwo::authenticate(token, clientDataHash)`. Replace raw slice indexing with `.get(..32).ok_or(...)` / `.get(32..).ok_or(...)`, returning `Error::Ctap(CtapError::Other)` on short keys. Validate decrypted pinUvAuthToken length at the boundary (16 bytes for PUAP1 per CTAP 2.2 §6.5.5.7 step 3.7, 32 bytes for PUAP2). Update PUAP1 mocks to use a spec-correct 16-byte token. Add regression tests for empty and 16-byte keys.
1 parent 234fdbc commit 99fa265

2 files changed

Lines changed: 73 additions & 6 deletions

File tree

libwebauthn/src/pin.rs

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
362362

363363
fn encrypt(&self, key: &[u8], plaintext: &[u8]) -> Result<Vec<u8>, Error> {
364364
// Discard the first 32 bytes of key. (This selects the AES-key portion of the shared secret.)
365-
let key = &key[32..];
365+
let key = key.get(32..).ok_or_else(|| {
366+
error!(
367+
key_len = key.len(),
368+
"key shorter than 32 bytes; cannot select AES-key portion"
369+
);
370+
Error::Ctap(CtapError::Other)
371+
})?;
366372

367373
// Let iv be a 16-byte, random bytestring.
368374
let iv: [u8; 16] = thread_rng().gen();
@@ -383,7 +389,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
383389

384390
fn decrypt(&self, key: &[u8], ciphertext: &[u8]) -> Result<Vec<u8>, Error> {
385391
// Discard the first 32 bytes of key. (This selects the AES-key portion of the shared secret.)
386-
let key = &key[32..];
392+
let key = key.get(32..).ok_or_else(|| {
393+
error!(
394+
key_len = key.len(),
395+
"key shorter than 32 bytes; cannot select AES-key portion"
396+
);
397+
Error::Ctap(CtapError::Other)
398+
})?;
387399

388400
// If demPlaintext is less than 16 bytes in length, return an error
389401
if ciphertext.len() < 16 {
@@ -409,7 +421,13 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
409421
fn authenticate(&self, key: &[u8], message: &[u8]) -> Result<Vec<u8>, Error> {
410422
// If key is longer than 32 bytes, discard the excess. (This selects the HMAC-key portion of the shared secret.
411423
// When key is the pinUvAuthToken, it is exactly 32 bytes long and thus this step has no effect.)
412-
let key = &key[..32];
424+
let key = key.get(..32).ok_or_else(|| {
425+
error!(
426+
key_len = key.len(),
427+
"key shorter than 32 bytes; cannot select HMAC-key portion"
428+
);
429+
Error::Ctap(CtapError::Other)
430+
})?;
413431

414432
// Return the result of computing HMAC-SHA-256 on key and message.
415433
hmac_sha256(key, message)
@@ -635,4 +653,37 @@ mod tests {
635653
let result = PinUvAuthProtocol::encapsulate(&proto, &key);
636654
assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
637655
}
656+
657+
#[test]
658+
fn proto_two_authenticate_rejects_empty_key() {
659+
let proto = PinUvAuthProtocolTwo::new();
660+
let result = proto.authenticate(&[], b"clientDataHash");
661+
assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
662+
}
663+
664+
#[test]
665+
fn proto_two_authenticate_rejects_short_key() {
666+
let proto = PinUvAuthProtocolTwo::new();
667+
let short_key = [0u8; 16];
668+
let result = proto.authenticate(&short_key, b"hello");
669+
assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
670+
}
671+
672+
#[test]
673+
fn proto_two_encrypt_rejects_short_key() {
674+
let proto = PinUvAuthProtocolTwo::new();
675+
let short_key = [0u8; 16];
676+
let plaintext = [0u8; 16];
677+
let result = proto.encrypt(&short_key, &plaintext);
678+
assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
679+
}
680+
681+
#[test]
682+
fn proto_two_decrypt_rejects_short_key() {
683+
let proto = PinUvAuthProtocolTwo::new();
684+
let short_key = [0u8; 16];
685+
let ct = [0u8; 32];
686+
let result = proto.decrypt(&short_key, &ct);
687+
assert!(matches!(result, Err(Error::Ctap(CtapError::Other))));
688+
}
638689
}

libwebauthn/src/webauthn/pin_uv_auth_token.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,22 @@ where
359359
let uv_auth_token =
360360
uv_proto.decrypt(&shared_secret, &encrypted_pin_uv_auth_token)?;
361361

362+
// pinUvAuthToken is 16 bytes for PUAP1 and 32 bytes for PUAP2.
363+
// Reject a shorter token before it is used as a key downstream.
364+
let min_token_len = match uv_proto.version() {
365+
Ctap2PinUvAuthProtocol::One => 16,
366+
Ctap2PinUvAuthProtocol::Two => 32,
367+
};
368+
if uv_auth_token.len() < min_token_len {
369+
error!(
370+
protocol = ?uv_proto.version(),
371+
token_len = uv_auth_token.len(),
372+
min_expected = min_token_len,
373+
"Decrypted pinUvAuthToken is shorter than required"
374+
);
375+
return Err(Error::Ctap(CtapError::Other));
376+
}
377+
362378
let token_identifier = Ctap2AuthTokenPermission::new(
363379
uv_proto.version(),
364380
ctap2_request.permissions(),
@@ -838,7 +854,7 @@ mod test {
838854
.unwrap();
839855
// We do here what the device would need to do, i.e. generate a new random
840856
// pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
841-
let token = [5; 32];
857+
let token = [5; 16];
842858
let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
843859
let pin_resp = CborResponse::new_success_from_slice(
844860
to_vec(&Ctap2ClientPinResponse {
@@ -1182,7 +1198,7 @@ mod test {
11821198
.unwrap();
11831199
// We do here what the device would need to do, i.e. generate a new random
11841200
// pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
1185-
let token = [5; 32];
1201+
let token = [5; 16];
11861202
let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
11871203
let pin_resp = CborResponse::new_success_from_slice(
11881204
to_vec(&Ctap2ClientPinResponse {
@@ -1322,7 +1338,7 @@ mod test {
13221338
.unwrap();
13231339
// We do here what the device would need to do, i.e. generate a new random
13241340
// pinUvAuthToken (here all 5's), then encrypt it using the shared_secret.
1325-
let token = [5; 32];
1341+
let token = [5; 16];
13261342
let encrypted_token = pin_protocol.encrypt(&shared_secret, &token).unwrap();
13271343
let pin_resp = CborResponse::new_success_from_slice(
13281344
to_vec(&Ctap2ClientPinResponse {

0 commit comments

Comments
 (0)