LoRaWAN: Memory corruption due to band mishandling

band_t structure in phy parameters was not being filled in properly
and we were spilling over the array boundary for bands.
In addition to that two utility functions are added to perform safety
checking taking in the frequency and filling out band information plus
boundry value checks.

Author: Hasnain Virk

features/lorawan/lorastack/phy/LoRaPHY.cpp

@@ -127,7 +127,7 @@ uint8_t LoRaPHY::request_new_channel(int8_t channel_id, channel_params_t* new_ch
             status &= 0xFC;
         }
     } else {
-
+        new_channel->band = lookup_band_for_frequency(new_channel->frequency);
         switch (add_channel(new_channel, channel_id)) {
             case LORAWAN_STATUS_OK:
             {
@@ -186,13 +186,13 @@ bool LoRaPHY::verify_channel_DR(uint8_t nb_channels, uint16_t* channel_mask,
     return false;
 }
 
-uint8_t LoRaPHY::val_in_range( int8_t value, int8_t min, int8_t max )
+bool LoRaPHY::val_in_range( int8_t value, int8_t min, int8_t max )
 {
     if ((value >= min) && (value <= max)) {
-        return 1;
+        return true;
     }
 
-    return 0;
+    return false;
 }
 
 bool LoRaPHY::disable_channel(uint16_t* channel_mask, uint8_t id,
@@ -733,6 +733,9 @@ void LoRaPHY::apply_cf_list(const uint8_t* payload, uint8_t size)
         }
 
         if (new_channel.frequency != 0) {
+            //lookup for band
+            new_channel.band = lookup_band_for_frequency(new_channel.frequency);
+
             // Try to add channel
             add_channel(&new_channel, channel_id);
         } else {
@@ -1065,21 +1068,30 @@ bool LoRaPHY::accept_tx_param_setup_req(uint8_t ul_dwell_time, uint8_t dl_dwell_
     return phy_params.accept_tx_param_setup_req;
 }
 
-bool LoRaPHY::verify_frequency(uint32_t freq)
+int LoRaPHY::lookup_band_for_frequency(uint32_t freq) const
 {
-    band_t *bands_table = (band_t *)phy_params.bands.table;
-
     // check all sub bands (if there are sub-bands) to check if the given
     // frequency falls into any of the frequency ranges
 
-    for (uint8_t i=0; i<phy_params.bands.size; i++) {
-        if (freq <= bands_table[i].higher_band_freq
-                 && freq >= bands_table[i].lower_band_freq) {
-            return true;
+    for (int band=0; band<phy_params.bands.size; band++) {
+        if (verify_frequency_for_band(freq, band)) {
+            return band;
         }
     }
 
-    return false;
+    return -1;
+}
+
+bool LoRaPHY::verify_frequency_for_band(uint32_t freq, uint8_t band) const
+{
+    band_t *bands_table = (band_t *)phy_params.bands.table;
+
+    if (freq <= bands_table[band].higher_band_freq
+            && freq >= bands_table[band].lower_band_freq) {
+        return true;
+    } else {
+        return false;
+    }
 }
 
 uint8_t LoRaPHY::dl_channel_request(uint8_t channel_id, uint32_t rx1_frequency)
@@ -1091,7 +1103,8 @@ uint8_t LoRaPHY::dl_channel_request(uint8_t channel_id, uint32_t rx1_frequency)
     uint8_t status = 0x03;
 
     // Verify if the frequency is supported
-    if (verify_frequency(rx1_frequency) == false) {
+    uint8_t band = lookup_band_for_frequency(rx1_frequency);
+    if (verify_frequency_for_band(rx1_frequency, band) == false) {
         status &= 0xFE;
     }
 
@@ -1263,7 +1276,7 @@ lorawan_status_t LoRaPHY::set_next_channel(channel_selection_params_t* params,
     return LORAWAN_STATUS_NO_CHANNEL_FOUND;
 }
 
-lorawan_status_t LoRaPHY::add_channel(channel_params_t* new_channel, uint8_t id)
+lorawan_status_t LoRaPHY::add_channel(const channel_params_t* new_channel, uint8_t id)
 {
     bool dr_invalid = false;
     bool freq_invalid = false;
@@ -1312,7 +1325,9 @@ lorawan_status_t LoRaPHY::add_channel(channel_params_t* new_channel, uint8_t id)
 
     // Check frequency
     if (!freq_invalid) {
-        if (verify_frequency(new_channel->frequency) == false) {
+        if (new_channel->band >= phy_params.bands.size
+                || verify_frequency_for_band(new_channel->frequency,
+                                             new_channel->band) == false) {
             freq_invalid = true;
         }
     }

features/lorawan/lorastack/phy/LoRaPHY.h

@@ -361,7 +361,7 @@ class LoRaPHY : private mbed::NonCopyable<LoRaPHY> {
      * @return LORAWAN_STATUS_OK if everything goes fine, negative error code
      *         otherwise.
      */
-    virtual lorawan_status_t add_channel(channel_params_t* new_channel, uint8_t id);
+    virtual lorawan_status_t add_channel(const channel_params_t* new_channel, uint8_t id);
 
     /** Removes a channel from the channel list.
      *
@@ -528,15 +528,19 @@ class LoRaPHY : private mbed::NonCopyable<LoRaPHY> {
     LoRaPHY(LoRaWANTimeHandler &lora_time);
 
     /**
-     * Verifies the given frequency.
+     * Looks up corresponding band for a frequency. Returns -1 if not in any band.
      */
-    virtual bool verify_frequency(uint32_t freq);
+    int lookup_band_for_frequency(uint32_t freq) const;
 
+    /**
+     * Verifies, if a frequency is within a given band.
+     */
+    virtual bool verify_frequency_for_band(uint32_t freq, uint8_t band) const;
 
     /**
      * Verifies, if a value is in a given range.
      */
-    uint8_t val_in_range(int8_t value, int8_t min, int8_t max);
+    bool val_in_range(int8_t value, int8_t min, int8_t max);
 
     /**
      * Verifies, if a datarate is available on an active channel.

features/lorawan/lorastack/phy/LoRaPHYKR920.cpp

@@ -339,7 +339,7 @@ int8_t LoRaPHYKR920::get_max_eirp(uint32_t freq)
 }
 
 
-bool LoRaPHYKR920::verify_frequency(uint32_t freq)
+bool LoRaPHYKR920::verify_frequency_for_band(uint32_t freq, uint8_t band) const
 {
     uint32_t tmp_freq = freq;
 

features/lorawan/lorastack/phy/LoRaPHYKR920.h

@@ -54,7 +54,7 @@ class LoRaPHYKR920 : public LoRaPHY {
     LoRaPHYKR920(LoRaWANTimeHandler &lora_time);
     virtual ~LoRaPHYKR920();
 
-    virtual bool verify_frequency(uint32_t freq);
+    virtual bool verify_frequency_for_band(uint32_t freq, uint8_t band) const;
 
     virtual bool tx_config(tx_config_params_t* config, int8_t* tx_power,
                            lorawan_time_t* tx_toa);