fix: extend check for header byte

Packet payloads should always include a header byte according to the
spec, but check that it actually does to better handle buggy clients
or bad actors.

Author: nyonson

protocol/src/io.rs

@@ -20,13 +20,24 @@ use crate::{
 use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 
 /// A decrypted BIP-324 payload.
+/// 
+/// # Invariants
+/// 
+/// The internal data vector must always contain at least one byte (the header byte).
+/// This invariant is maintained by the decrypt functions which validate that
+/// ciphertext contains at least `NUM_TAG_BYTES + NUM_HEADER_BYTES` before
+/// attempting decryption.
 pub struct Payload {
     data: Vec<u8>,
 }
 
 impl Payload {
     /// Create a new payload from complete decrypted data (including header byte).
+    /// 
+    /// The data must contain at least one byte (the header). This is guaranteed
+    /// by the decrypt functions, but can be asserted in debug builds.
     pub fn new(data: Vec<u8>) -> Self {
+        debug_assert!(!data.is_empty(), "Payload data must contain at least the header byte");
         Self { data }
     }
 

protocol/src/lib.rs

@@ -360,7 +360,7 @@ impl InboundCipher {
     ) -> Result<(PacketType, &'a [u8]), Error> {
         let auth = aad.unwrap_or_default();
         // Check minimum size of ciphertext.
-        if ciphertext.len() < NUM_TAG_BYTES {
+        if ciphertext.len() < NUM_TAG_BYTES + NUM_HEADER_BYTES {
             return Err(Error::CiphertextTooSmall);
         }
         let (msg, tag) = ciphertext.split_at_mut(ciphertext.len() - NUM_TAG_BYTES);
@@ -400,7 +400,7 @@ impl InboundCipher {
     ) -> Result<PacketType, Error> {
         let auth = aad.unwrap_or_default();
         // Check minimum size of ciphertext.
-        if ciphertext.len() < NUM_TAG_BYTES {
+        if ciphertext.len() < NUM_TAG_BYTES + NUM_HEADER_BYTES {
             return Err(Error::CiphertextTooSmall);
         }
         let (msg, tag) = ciphertext.split_at(ciphertext.len() - NUM_TAG_BYTES);
@@ -763,6 +763,40 @@ mod tests {
         assert_eq!(message2, plaintext2[1..].to_vec()); // Skip header byte
     }
 
+    #[test]
+    fn test_decrypt_min_length() {
+        // Test that decrypt properly validates minimum ciphertext length.
+        let alice =
+            SecretKey::from_str("61062ea5071d800bbfd59e2e8b53d47d194b095ae5a4df04936b49772ef0d4d7")
+                .unwrap();
+        let elliswift_alice = ElligatorSwift::from_str("ec0adff257bbfe500c188c80b4fdd640f6b45a482bbc15fc7cef5931deff0aa186f6eb9bba7b85dc4dcc28b28722de1e3d9108b985e2967045668f66098e475b").unwrap();
+        let elliswift_bob = ElligatorSwift::from_str("a4a94dfce69b4a2a0a099313d10f9f7e7d649d60501c9e1d274c300e0d89aafaffffffffffffffffffffffffffffffffffffffffffffffffffffffff8faf88d5").unwrap();
+        let session_keys = SessionKeyMaterial::from_ecdh(
+            elliswift_alice,
+            elliswift_bob,
+            alice,
+            ElligatorSwiftParty::A,
+            Network::Bitcoin,
+        )
+        .unwrap();
+        let mut alice_cipher = CipherSession::new(session_keys, Role::Initiator);
+
+        // Test with ciphertext that is exactly NUM_TAG_BYTES (should fail).
+        let too_small = vec![0u8; NUM_TAG_BYTES];
+        let mut plaintext_buffer = vec![0u8; 100];
+        let result = alice_cipher
+            .inbound()
+            .decrypt(&too_small, &mut plaintext_buffer, None);
+        assert_eq!(result, Err(Error::CiphertextTooSmall));
+
+        // Test decrypt_in_place with same minimum length checks.
+        let mut too_small = vec![0u8; NUM_TAG_BYTES];
+        let result = alice_cipher
+            .inbound()
+            .decrypt_in_place(&mut too_small, None);
+        assert_eq!(result, Err(Error::CiphertextTooSmall));
+    }
+
     #[test]
     fn test_fuzz_packets() {
         let mut rng = rand::thread_rng();