Merge pull request #437 from helium/madninja/invalid_packets

* Filter out non parseable lorawan packets
* Explicitly filter out proprietary packets in check
* ignore join_accepts as uplinks
* Add major version check at frame parse

* Add minimum potential beacon length check

* Handle SF5 and SF6 Lora datarates

* More precise beacon size check

Cargo.toml

@@ -16,50 +16,69 @@ members = ["lorawan"]
 
 [workspace.dependencies]
 byteorder = "1"
-serde = {version = "1", features = ["rc", "derive"]}
-rust_decimal = {version = "1", features = ["serde-with-float"]}
-helium-proto = { git = "https://github.com/helium/proto", branch="master", features=["services"]}
+serde = { version = "1", features = ["rc", "derive"] }
+rust_decimal = { version = "1", features = ["serde-with-float"] }
+helium-proto = { git = "https://github.com/helium/proto", branch = "master", features = [
+    "services",
+] }
 rand = "0.8"
 base64 = ">=0.21"
 sha2 = "0"
 thiserror = "1.0"
 prost = "0"
 
 [dependencies]
-clap = {version = "4", default-features=false, features = ["derive", "help", "std", "error-context"]}
+clap = { version = "4", default-features = false, features = [
+    "derive",
+    "help",
+    "std",
+    "error-context",
+] }
 semver = "0"
-config = {version="0", default-features=false, features=["toml"]}
-serde = {workspace = true}
+config = { version = "0", default-features = false, features = ["toml"] }
+serde = { workspace = true }
 serde_json = "1"
 serde_urlencoded = "*"
 http-serde = "1"
-tokio = { version="1", default-features=false, features=["macros", "signal", "rt", "time", "sync"] }
-tokio-stream = {version="0", default-features=false }
+tokio = { version = "1", default-features = false, features = [
+    "macros",
+    "signal",
+    "rt",
+    "time",
+    "sync",
+] }
+tokio-stream = { version = "0", default-features = false }
 futures = "*"
 triggered = "0.1"
 tracing = "0"
-tracing-subscriber = {version = "0", default-features = false, features = ["smallvec", "fmt", "std"]}
+tracing-subscriber = { version = "0", default-features = false, features = [
+    "smallvec",
+    "fmt",
+    "std",
+] }
 tracing-appender = "0"
-thiserror = {workspace = true}
-rand = {workspace = true}
-prost = {workspace = true}
+thiserror = { workspace = true }
+rand = { workspace = true }
+prost = { workspace = true }
 tonic = "0"
 http = "*"
-sha2 = {workspace = true}
-base64 = {workspace = true}
-helium-proto = {workspace = true}
+sha2 = { workspace = true }
+base64 = { workspace = true }
+helium-proto = { workspace = true }
 signature = "2"
 async-trait = "0"
 angry-purple-tiger = "0"
 lorawan = { package = "lorawan", path = "lorawan" }
-beacon = { git = "https://github.com/helium/proto", branch="master" }
-exponential-backoff = {git = "https://github.com/yoshuawuyts/exponential-backoff", branch = "master"}
-semtech-udp = { version = ">=0.10.7", default-features=false, features=["server"] }
+beacon = { git = "https://github.com/helium/proto", branch = "master" }
+exponential-backoff = { git = "https://github.com/yoshuawuyts/exponential-backoff", branch = "master" }
+semtech-udp = { version = ">=0.10.9", default-features = false, features = [
+    "server",
+] }
 helium-crypto = "0.6"
 
 [features]
-default = [ "ecc608" ]
-ecc608 = [ "helium-crypto/ecc608" ]
+default = ["ecc608"]
+ecc608 = ["helium-crypto/ecc608"]
 tpm = ["helium-crypto/tpm"]
 
 [profile.release]

lorawan/src/error.rs

@@ -3,6 +3,7 @@ use std::{error::Error, fmt, io};
 #[derive(Debug)]
 pub enum LoraWanError {
     InvalidPacketType(u8),
+    InvalidPacketVersion(u8),
     InvalidFPortForFopts,
     InvalidPacketSize(super::MType, usize),
     Io(io::Error),
@@ -12,6 +13,7 @@ impl fmt::Display for LoraWanError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             LoraWanError::InvalidPacketType(v) => write!(f, "Invalid packet type: {v:#02x}"),
+            LoraWanError::InvalidPacketVersion(v) => write!(f, "Invalid packet version: {v:#02x}"),
             LoraWanError::InvalidFPortForFopts => write!(f, "Invalid: fport 0 with fopts"),
             LoraWanError::InvalidPacketSize(mtype, s) => {
                 write!(f, "Invalid packet size {s} for type {mtype:?}")

lorawan/src/lib.rs

@@ -100,6 +100,10 @@ impl PHYPayload {
 
     pub fn read(direction: Direction, reader: &mut dyn Buf) -> Result<Self, LoraWanError> {
         let mhdr = MHDR::read(reader)?;
+        let version = mhdr.major();
+        if version != 0 {
+            return Err(LoraWanError::InvalidPacketVersion(version));
+        }
         let packet_type = mhdr.mtype();
         let mut data = reader.copy_to_bytes(reader.remaining());
 

src/gateway.rs

@@ -140,7 +140,12 @@ impl Gateway {
                     Ok(packet) if packet.is_potential_beacon() => {
                         self.handle_potential_beacon(packet).await;
                     }
-                    Ok(packet) => self.handle_uplink(packet, Instant::now()).await,
+                    Ok(packet) if packet.is_uplink() => {
+                        self.handle_uplink(packet, Instant::now()).await
+                    }
+                    Ok(packet) => {
+                        info!(%packet, "ignoring non-uplink packet");
+                    }
                     Err(err) => {
                         warn!(%err, "ignoring push_data");
                     }

src/packet.rs

@@ -113,7 +113,23 @@ impl PacketUp {
 
     pub fn is_potential_beacon(&self) -> bool {
         Self::parse_header(self.payload())
-            .map(|header| header.mtype() == lorawan::MType::Proprietary)
+            .map(|header| {
+                header.mtype() == lorawan::MType::Proprietary
+                    && self.payload().len() == beacon::BEACON_PAYLOAD_SIZE + Self::header_size()
+            })
+            .unwrap_or(false)
+    }
+
+    pub fn is_uplink(&self) -> bool {
+        // An uplinkable packet is a parseable lorawan uplink frame which is not
+        // a proprietary frame
+        Self::parse_frame(Direction::Uplink, self.payload())
+            .map(|frame| {
+                !matches!(
+                    frame,
+                    PHYPayloadFrame::Proprietary(_) | PHYPayloadFrame::JoinAccept(_),
+                )
+            })
             .unwrap_or(false)
     }
 
@@ -126,6 +142,10 @@ impl PacketUp {
         lorawan::MHDR::read(&mut Cursor::new(payload)).map_err(Error::from)
     }
 
+    pub fn header_size() -> usize {
+        std::mem::size_of::<MHDR>()
+    }
+
     pub fn parse_frame(direction: lorawan::Direction, payload: &[u8]) -> Result<PHYPayloadFrame> {
         use std::io::Cursor;
         lorawan::PHYPayload::read(direction, &mut Cursor::new(payload))
@@ -266,6 +286,10 @@ pub(crate) mod datarate {
             (SpreadingFactor::SF9, Bandwidth::BW500) => ProtoRate::Sf9bw500,
             (SpreadingFactor::SF8, Bandwidth::BW500) => ProtoRate::Sf8bw500,
             (SpreadingFactor::SF7, Bandwidth::BW500) => ProtoRate::Sf7bw500,
+
+            (SpreadingFactor::SF6, _) | (SpreadingFactor::SF5, _) => {
+                return Err(DecodeError::invalid_data_rate(rate.to_string()))
+            }
         };
         Ok(rate)
     }