From 44815d619bffdb0b84df911aef96a286eb8251eb Mon Sep 17 00:00:00 2001
From: Antoni Spaanderman <56turtle56@gmail.com>
Date: Tue, 14 Nov 2023 11:32:32 +0100
Subject: [PATCH] properly handle too long topic (should only happen with a
 bitcoind bug) + add tests for it

---
 src/error.rs   | 18 ++++++++++++++++--
 src/message.rs | 40 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 55e4718..c350a25 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,6 +1,6 @@
 use crate::message::{DATA_MAX_LEN, SEQUENCE_LEN, TOPIC_MAX_LEN};
 use bitcoin::consensus;
-use core::fmt;
+use core::{cmp::min, fmt};
 
 pub type Result<T> = core::result::Result<T, Error>;
 
@@ -17,6 +17,20 @@ pub enum Error {
     Zmq(zmq::Error),
 }
 
+impl Error {
+    /// Returns the (invalid) topic as a byte slice (as this might not always be valid UTF-8). If
+    /// this error is not an [`Error::InvalidTopic`], [`None`] is returned. The real length is also
+    /// returned, if this is higher that the length of the slice, the data was truncated to fit
+    /// directly in the object, instead of with a heap allocation.
+    pub fn invalid_topic_data(&self) -> Option<(&[u8], usize)> {
+        if let Self::InvalidTopic(len, buf) = self {
+            Some((&buf[..min(*len, buf.len())], *len))
+        } else {
+            None
+        }
+    }
+}
+
 impl From<zmq::Error> for Error {
     #[inline]
     fn from(value: zmq::Error) -> Self {
@@ -66,7 +80,7 @@ impl fmt::Display for Error {
                 write!(
                     f,
                     "invalid message topic '{}'{}",
-                    String::from_utf8_lossy(&topic[0..*len]),
+                    String::from_utf8_lossy(&topic[..min(*len, topic.len())]),
                     if *len > TOPIC_MAX_LEN {
                         " (truncated)"
                     } else {
diff --git a/src/message.rs b/src/message.rs
index 8c3e9d0..88a7df3 100644
--- a/src/message.rs
+++ b/src/message.rs
@@ -8,7 +8,7 @@ use bitcoin::{
     hashes::Hash,
     Block, BlockHash, Transaction, Txid,
 };
-use core::fmt;
+use core::{cmp::min, fmt};
 
 pub const TOPIC_MAX_LEN: usize = 9;
 pub const DATA_MAX_LEN: usize = MAX_BLOCK_WEIGHT as usize;
@@ -134,7 +134,8 @@ impl Message {
             _ => {
                 let mut buf = [0; TOPIC_MAX_LEN];
 
-                buf[0..topic.len()].copy_from_slice(topic);
+                buf[..min(TOPIC_MAX_LEN, topic.len())]
+                    .copy_from_slice(&topic[..min(TOPIC_MAX_LEN, topic.len())]);
 
                 return Err(Error::InvalidTopic(topic.len(), buf));
             }
@@ -257,4 +258,39 @@ mod tests {
 
         assert_eq!(msg.serialize_to_vecs(), to_deserialize);
     }
+
+    #[test]
+    fn test_deserialization_errors() {
+        let to_deserialize = [b"abc" as &[u8], &[], &[0x05, 0x00, 0x00, 0x00], b"garbage"];
+
+        assert!(matches!(
+            Message::from_multipart(&to_deserialize[..0]),
+            Err(Error::InvalidMutlipartLength(0))
+        ));
+        assert!(matches!(
+            Message::from_multipart(&to_deserialize[..1]),
+            Err(Error::InvalidMutlipartLength(1))
+        ));
+        assert!(matches!(
+            Message::from_multipart(&to_deserialize[..2]),
+            Err(Error::InvalidMutlipartLength(2))
+        ));
+        assert_eq!(
+            Message::from_multipart(&to_deserialize[..3])
+                .expect_err("expected invalid topic")
+                .invalid_topic_data(),
+            Some((b"abc" as &[u8], 3))
+        );
+        assert!(matches!(
+            Message::from_multipart(&to_deserialize[..4]),
+            Err(Error::InvalidMutlipartLength(4))
+        ));
+
+        assert_eq!(
+            Message::from_multipart(&[b"hashblock!" as &[u8], &[], &[0x06, 0x00, 0x00, 0x00]])
+                .expect_err("expected invalid topic")
+                .invalid_topic_data(),
+            Some((b"hashblock" as &[u8], 10))
+        )
+    }
 }