Просмотр исходного кода

src/cyw43_ctrl: Handle errors and buffer overflow when getting STA MACs.

- cyw43_ll_wifi_ap_get_stas does not perform checks on the return value of
  cyw43_get_ioctl.  On error (e.g. timeout), invalid num_stas values can be
  used in the following memcpy call, leading to crashes.

- cyw43_ll_wifi_ap_get_stas now returns the return code of cyw43_do_ioctl
  on error.

- Perform bounds checks before using memcpy.  This requires the  parameter
  num_stas to contain the size (in MAC addresses) of the supplied buffer.

- Associated functions in cyw43_ctrl.c set num_stas to 0 on error.
Reinier Heeres 2 лет назад
Родитель
Сommit
8dcf3c7582
3 измененных файлов с 24 добавлено и 7 удалено
  1. 5 3
      src/cyw43.h
  2. 4 1
      src/cyw43_ctrl.c
  3. 15 3
      src/cyw43_ll.c

+ 5 - 3
src/cyw43.h

@@ -474,6 +474,7 @@ static inline void cyw43_wifi_ap_set_auth(cyw43_t *self, uint32_t auth) {
  *
  * \param self the driver state object. This should always be \c &cyw43_state
  * \param max_stas Returns the maximum number of devices (STAs) that can be connected to the access point
+ * (set to 0 on error)
  */
 void cyw43_wifi_ap_get_max_stas(cyw43_t *self, int *max_stas);
 
@@ -484,9 +485,10 @@ void cyw43_wifi_ap_get_max_stas(cyw43_t *self, int *max_stas);
  * connected to the wifi access point.
  *
  * \param self the driver state object. This should always be \c &cyw43_state
- * \param num_stas Returns the number of devices (STA) connected to the access point
- * \param macs Returns the mac addresses of devies (STA) connected to the access point.
- * The supplied buffer should have enough room for 6 bytes per mac address.
+ * \param num_stas Caller must provide the number of MACs that will fit in the macs buffer;
+ * The supplied buffer should have enough room for 6 bytes per MAC address.
+ * Returns the number of devices (STA) connected to the access point.
+ * \param macs Returns up to num_stas MAC addresses of devices (STA) connected to the access point.
  * Call \ref cyw43_wifi_ap_get_max_stas to determine how many mac addresses can be returned.
  */
 void cyw43_wifi_ap_get_stas(cyw43_t *self, int *num_stas, uint8_t *macs);

+ 4 - 1
src/cyw43_ctrl.c

@@ -707,7 +707,10 @@ void cyw43_wifi_ap_get_stas(cyw43_t *self, int *num_stas, uint8_t *macs) {
         return;
     }
 
-    cyw43_ll_wifi_ap_get_stas(&self->cyw43_ll, num_stas, macs);
+    ret = cyw43_ll_wifi_ap_get_stas(&self->cyw43_ll, num_stas, macs);
+    if (ret != 0) {
+        *num_stas = 0;
+    }
     CYW43_THREAD_EXIT;
 }
 

+ 15 - 3
src/cyw43_ll.c

@@ -2280,7 +2280,13 @@ int cyw43_ll_wifi_ap_get_stas(cyw43_ll_t *self_in, int *num_stas, uint8_t *macs)
     memcpy(buf, "maxassoc\x00", 9);
     cyw43_put_le32(buf + 9, WWD_AP_INTERFACE);
     cyw43_do_ioctl(self, SDPCM_GET, WLC_GET_VAR, 9 + 4, buf, WWD_STA_INTERFACE);
+    int err = cyw43_do_ioctl(self, SDPCM_GET, WLC_GET_VAR, 9 + 4, buf, WWD_STA_INTERFACE);
+    if (err != 0) {
+        return err;
+    }
     uint32_t maxassoc = cyw43_get_le32(buf);
+    uint32_t max_macs_buf = (sizeof(self->spid_buf) - SDPCM_HEADER_LEN - 16 - 4) / 6;
+    maxassoc = MIN(maxassoc, max_macs_buf);
 
     if (macs == NULL) {
         // Return just the maximum number of STAs
@@ -2288,11 +2294,17 @@ int cyw43_ll_wifi_ap_get_stas(cyw43_ll_t *self_in, int *num_stas, uint8_t *macs)
         return 0;
     }
 
+    // Make sure all MACs will fit in buffer; size specified in num_stas
+    maxassoc = MIN(maxassoc, (uint32_t)*num_stas);
+
     // Get associated STAs
     cyw43_put_le32(buf, maxassoc);
-    cyw43_do_ioctl(self, SDPCM_GET, WLC_GET_ASSOCLIST, 4 + maxassoc * 6, buf, WWD_AP_INTERFACE);
-
-    *num_stas = cyw43_get_le32(buf);
+    err = cyw43_do_ioctl(self, SDPCM_GET, WLC_GET_ASSOCLIST, 4 + maxassoc * 6, buf, WWD_AP_INTERFACE);
+    if (err != 0) {
+        return err;
+    }
+    uint32_t stas_connected = cyw43_get_le32(buf);
+    *num_stas = MIN(stas_connected, maxassoc);
     memcpy(macs, buf + 4, *num_stas * 6);
     return 0;
 }