Skip to content

Commit 82e5acf

Browse files
fix(cable): bound decrypt_frame indexing on malformed Noise plaintext
After Noise transport decryption, the last byte of the plaintext is read as a 0..=255 padding length and the frame is truncated by `padding_len + 1`. Two crash inputs are reachable from any legitimate-but-malicious paired peer: 1. Empty plaintext (Noise transport accepts a 16-byte AEAD-tag-only ciphertext, decrypting to 0 bytes): reading `frame[len - 1]` underflows to `usize::MAX` and panics with `index out of bounds`. 2. Under-padded plaintext (e.g., `[0x05]`): `1 - 6` panics in debug builds and silently wraps in release, so the subsequent `truncate(huge)` no-ops and the malformed plaintext is parsed downstream. Extract the padding-stripping into a `strip_frame_padding` helper that uses `.last()` and `.checked_sub`, returning `Error::Transport(TransportError::InvalidFraming)` on either edge case. Add regression tests for the empty and overlong-padding inputs plus a happy-path check.
1 parent 10ee804 commit 82e5acf

1 file changed

Lines changed: 54 additions & 2 deletions

File tree

libwebauthn/src/transport/cable/tunnel.rs

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,31 @@ async fn connection_recv_binary_frame(message: Message) -> Result<Option<Vec<u8>
523523
}
524524
}
525525

526+
/// Strip the trailing padding-length byte and `padding_len` bytes of padding
527+
/// from a decrypted Noise transport frame, returning `InvalidFraming` on an
528+
/// empty plaintext or a declared padding length that exceeds the frame.
529+
fn strip_frame_padding(mut decrypted_frame: Vec<u8>) -> Result<Vec<u8>, Error> {
530+
let padding_len = match decrypted_frame.last() {
531+
Some(&b) => b as usize,
532+
None => {
533+
error!("Decrypted frame is empty; cannot read padding length");
534+
return Err(Error::Transport(TransportError::InvalidFraming));
535+
}
536+
};
537+
let new_len = decrypted_frame
538+
.len()
539+
.checked_sub(padding_len + 1)
540+
.ok_or_else(|| {
541+
error!(
542+
frame_len = decrypted_frame.len(),
543+
padding_len, "Padding length exceeds frame length"
544+
);
545+
Error::Transport(TransportError::InvalidFraming)
546+
})?;
547+
decrypted_frame.truncate(new_len);
548+
Ok(decrypted_frame)
549+
}
550+
526551
async fn decrypt_frame(
527552
encrypted_frame: Vec<u8>,
528553
noise_state: &mut TunnelNoiseState,
@@ -543,8 +568,7 @@ async fn decrypt_frame(
543568
}
544569
}
545570

546-
let padding_len = decrypted_frame[decrypted_frame.len() - 1] as usize;
547-
decrypted_frame.truncate(decrypted_frame.len() - (padding_len + 1));
571+
let decrypted_frame = strip_frame_padding(decrypted_frame)?;
548572
trace!(
549573
?decrypted_frame,
550574
decrypted_frame_len = decrypted_frame.len(),
@@ -795,4 +819,32 @@ mod tests {
795819
}
796820

797821
// TODO: test the non-known case
822+
823+
#[test]
824+
fn strip_frame_padding_rejects_empty() {
825+
let result = strip_frame_padding(Vec::new());
826+
assert!(matches!(
827+
result,
828+
Err(Error::Transport(TransportError::InvalidFraming))
829+
));
830+
}
831+
832+
#[test]
833+
fn strip_frame_padding_rejects_overlong_padding() {
834+
// Length 1 + declared padding of 5 -> would require subtracting 6 from 1.
835+
let frame = vec![0x05u8];
836+
let result = strip_frame_padding(frame);
837+
assert!(matches!(
838+
result,
839+
Err(Error::Transport(TransportError::InvalidFraming))
840+
));
841+
}
842+
843+
#[test]
844+
fn strip_frame_padding_strips_normal_padding() {
845+
// 4 bytes of payload, 3 bytes of zero padding, then padding-length 3.
846+
let frame = vec![0xAA, 0xBB, 0xCC, 0xDD, 0x00, 0x00, 0x00, 0x03];
847+
let stripped = strip_frame_padding(frame).unwrap();
848+
assert_eq!(stripped, vec![0xAA, 0xBB, 0xCC, 0xDD]);
849+
}
798850
}

0 commit comments

Comments
 (0)