From a4358e49de520e901e6a38082f24edc4f9b43855 Mon Sep 17 00:00:00 2001
From: Marc Nijdam <marc_nijdam@mac.com>
Date: Wed, 16 Aug 2023 14:15:31 -0600
Subject: [PATCH] Add FHDR length validation (#449)

* Add FHDR length validation
* Fixes join accept parsing validation size
* Fhdr::write: fix double output of dev_addr
* provide unit test for invalid fopts_len

---------

Co-authored-by: lthiery <thiery.louis@gmail.com>
---
 lorawan/src/lib.rs | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/lorawan/src/lib.rs b/lorawan/src/lib.rs
index 692c941f..f5d6063a 100644
--- a/lorawan/src/lib.rs
+++ b/lorawan/src/lib.rs
@@ -233,6 +233,8 @@ pub struct Fhdr {
     pub fopts: Bytes,
 }
 
+const FHDR_MIN_SIZE: usize = size_of::<u32>() + FCTRL_SIZE + size_of::<u16>();
+
 impl fmt::Debug for Fhdr {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> result::Result<(), fmt::Error> {
         f.debug_struct("Fhdr")
@@ -250,9 +252,23 @@ impl Fhdr {
         payload_type: MType,
         reader: &mut dyn Buf,
     ) -> Result<Self, LoraWanError> {
+        // Check minimum length requirements
+        if reader.remaining() < FHDR_MIN_SIZE {
+            return Err(LoraWanError::InvalidPacketSize(
+                payload_type,
+                reader.remaining(),
+            ));
+        }
         let dev_addr = reader.get_u32_le();
         let fctrl = FCtrl::read(direction, payload_type, reader)?;
         let fcnt = reader.get_u16_le();
+        // Ensure indicated fopts data is available
+        if reader.remaining() < fctrl.fopts_len() {
+            return Err(LoraWanError::InvalidPacketSize(
+                payload_type,
+                reader.remaining(),
+            ));
+        }
         let fopts = reader.copy_to_bytes(fctrl.fopts_len());
         let res = Self {
             dev_addr,
@@ -267,7 +283,6 @@ impl Fhdr {
         let mut written = 0;
         output.put_u32_le(self.dev_addr);
         written += size_of::<u32>();
-        output.put_u32_le(self.dev_addr);
         written += self.fctrl.write(output)?;
         output.put_u16_le(self.fcnt);
         written += size_of::<u16>();
@@ -343,6 +358,8 @@ pub enum FCtrl {
     Downlink(FCtrlDownlink),
 }
 
+pub const FCTRL_SIZE: usize = size_of::<u8>();
+
 impl FCtrl {
     pub fn fopts_len(&self) -> usize {
         match self {
@@ -527,7 +544,7 @@ pub struct JoinAccept {
     // cf_list: Option<CFList>,
 }
 
-const JOIN_ACCEPT_SIZE: usize = 3 * size_of::<u8>() + size_of::<u32>() + 2 * size_of::<u8>();
+const JOIN_ACCEPT_SIZE: usize = 6 * size_of::<u8>() + size_of::<u32>() + 2 * size_of::<u8>();
 
 impl JoinAccept {
     pub fn read(reader: &mut dyn Buf) -> Result<Self, LoraWanError> {
@@ -587,6 +604,28 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_fopts_len_error() {
+        // FCtrl indicates 8 bytes in FOpts but we will pass empty FOpts
+        let mut fctrl_uplink = FCtrlUplink(0);
+        fctrl_uplink.set_fopts_len(8);
+        let fhdr = Fhdr {
+            dev_addr: 0,
+            fcnt: 0,
+            fctrl: FCtrl::Uplink(fctrl_uplink),
+            fopts: Bytes::new(),
+        };
+        let mut buffer = Vec::new();
+        fhdr.write(&mut buffer).unwrap();
+        // Coerce to Bytes to hit the Bytes implementation
+        let mut buffer = bytes::Bytes::from(buffer);
+        let read = Fhdr::read(Direction::Uplink, MType::UnconfirmedUp, &mut buffer);
+        match read {
+            Err(LoraWanError::InvalidPacketSize(MType::UnconfirmedUp, 0)) => (),
+            _ => panic!("Error was expected! There was none or it was the wrong type"),
+        }
+    }
+
     impl TryFrom<&[u8]> for Routing {
         type Error = LoraWanError;
         fn try_from(value: &[u8]) -> Result<Self, Self::Error> {