[PATCH] wpa_supplicant: Fix mixing enum and int type in config parser

Chien Wong m at xv97.com
Tue Feb 27 05:31:19 PST 2024


Hi, everyone

The problem addressed by the patch is straightforward: within 
wpa_supplicant/config.c,

there are two tables, ssid_fields[] and global_fields[], utilized for 
parsing configuration files.

These tables contain information such as configuration names, parsers, 
member offsets

in the structure, and so on. Upon investigation, it was discovered that 
numerous enum

members are being parsed by int parser, which may lead to potential bugs.

Enums can differ in size from ints, potentially resulting in corruption 
of the relevant

structure.

This patch seeks advice on a potential solution. While the current 
approach may not be

optimal, adding something like ENUM(), ENUM_RANGE() to parse the enums 
could be

an alternative.

 From ec2fc4565da50ee7a140087ba91a7f3dba325a45 Mon Sep 17 00:00:00 2001
From: Chien Wong <m at xv97.com>
Date: Mon, 26 Feb 2024 22:08:56 +0800
Subject: [PATCH] wpa_supplicant: Fix mixing enum and int type in config 
parser

Currently, many enum members are handled as ints in the config parser.
C does not guarantee enum and int are of the same size. What's worse, when
-fshort-enums is enabled on GCC, enum can be as short as char.
If sizeof(enum) < sizeof(int), buffer overflow will occur when parsing 
config.
Fix this by changing related enum members to ints.

Signed-off-by: Chien Wong <m at xv97.com>
---
  wpa_supplicant/config.h      | 98 ++++++++++++++++++++++--------------
  wpa_supplicant/config_ssid.h | 31 +++++++++---
  2 files changed, 83 insertions(+), 46 deletions(-)

diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h
index 8981305c2..994a85489 100644
--- a/wpa_supplicant/config.h
+++ b/wpa_supplicant/config.h
@@ -441,6 +441,45 @@ struct wpa_cred {
  #define CFG_CHANGED_BGSCAN BIT(20)
  #define CFG_CHANGED_FT_PREPEND_PMKID BIT(21)

+/**
+ * p2p_go_freq_change_policy - The GO frequency change policy
+ *
+ * This controls the behavior of the GO when there is a change in the
+ * map of the currently used frequencies in case more than one channel
+ * is supported.
+ *
+ * @P2P_GO_FREQ_MOVE_SCM: Prefer working in a single channel mode if
+ * possible. In case the GO is the only interface using its frequency
+ * and there are other station interfaces on other frequencies, the GO
+ * will migrate to one of these frequencies.
+ *
+ * @P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS: Same as P2P_GO_FREQ_MOVE_SCM,
+ * but a transition is possible only in case one of the other used
+ * frequencies is one of the frequencies in the intersection of the
+ * frequency list of the local device and the peer device.
+ *
+ * @P2P_GO_FREQ_MOVE_STAY: Prefer to stay on the current frequency.
+ *
+ * @P2P_GO_FREQ_MOVE_SCM_ECSA: Same as
+ * P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS but a transition is possible only
+ * if all the group members advertise eCSA support.
+ */
+enum p2p_go_freq_change_policy {
+    P2P_GO_FREQ_MOVE_SCM = 0,
+    P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS = 1,
+    P2P_GO_FREQ_MOVE_STAY = 2,
+    P2P_GO_FREQ_MOVE_SCM_ECSA = 3,
+    P2P_GO_FREQ_MOVE_MAX = P2P_GO_FREQ_MOVE_SCM_ECSA,
+};
+
+enum mld_connect_band_pref {
+    MLD_CONNECT_BAND_PREF_AUTO = 0,
+    MLD_CONNECT_BAND_PREF_2GHZ = 1,
+    MLD_CONNECT_BAND_PREF_5GHZ = 2,
+    MLD_CONNECT_BAND_PREF_6GHZ = 3,
+    MLD_CONNECT_BAND_PREF_MAX = 4,
+};
+
  /**
   * struct wpa_config - wpa_supplicant configuration data
   *
@@ -448,6 +487,9 @@ struct wpa_cred {
   * data. In many cases, there is only one struct wpa_config instance, 
but if
   * more than one network interface is being controlled, one instance 
is used
   * for each.
+ *
+ * Warning: Pay attention to using enums in the structure as treating 
enums as ints
+ * in the config parser could be dangerous if sizeof(enum) != sizeof(int).
   */
  struct wpa_config {
      /**
@@ -888,33 +930,9 @@ struct wpa_config {
      /**
       * p2p_go_freq_change_policy - The GO frequency change policy
       *
-     * This controls the behavior of the GO when there is a change in the
-     * map of the currently used frequencies in case more than one channel
-     * is supported.
-     *
-     * @P2P_GO_FREQ_MOVE_SCM: Prefer working in a single channel mode if
-     * possible. In case the GO is the only interface using its frequency
-     * and there are other station interfaces on other frequencies, the GO
-     * will migrate to one of these frequencies.
-     *
-     * @P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS: Same as P2P_GO_FREQ_MOVE_SCM,
-     * but a transition is possible only in case one of the other used
-     * frequencies is one of the frequencies in the intersection of the
-     * frequency list of the local device and the peer device.
-     *
-     * @P2P_GO_FREQ_MOVE_STAY: Prefer to stay on the current frequency.
-     *
-     * @P2P_GO_FREQ_MOVE_SCM_ECSA: Same as
-     * P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS but a transition is possible only
-     * if all the group members advertise eCSA support.
+     * Enumerations are defined in enum p2p_go_freq_change_policy.
       */
-    enum {
-        P2P_GO_FREQ_MOVE_SCM = 0,
-        P2P_GO_FREQ_MOVE_SCM_PEER_SUPPORTS = 1,
-        P2P_GO_FREQ_MOVE_STAY = 2,
-        P2P_GO_FREQ_MOVE_SCM_ECSA = 3,
-        P2P_GO_FREQ_MOVE_MAX = P2P_GO_FREQ_MOVE_SCM_ECSA,
-    } p2p_go_freq_change_policy;
+    int p2p_go_freq_change_policy;

  #define DEFAULT_P2P_GO_FREQ_MOVE P2P_GO_FREQ_MOVE_STAY

@@ -1261,12 +1279,13 @@ struct wpa_config {
      /**
       * pmf - Whether to enable/require PMF by default
       *
+     * Enumerations are defined in enum mfp_options.
       * By default, PMF is disabled unless enabled by the per-network
       * ieee80211w=1 or ieee80211w=2 parameter. pmf=1/2 can be used to 
change
       * this default behavior for RSN network (this is not applicable for
       * non-RSN cases).
       */
-    enum mfp_options pmf;
+    int pmf;

      /**
       * sae_check_mfp - Whether to limit SAE based on PMF capabilities
@@ -1299,11 +1318,12 @@ struct wpa_config {

      /**
       * sae_pwe - SAE mechanism for PWE derivation
+     * Enumerations are defined in enum sae_pwe.
       * 0 = hunting-and-pecking loop only
       * 1 = hash-to-element only
       * 2 = both hunting-and-pecking loop and hash-to-element enabled
       */
-    enum sae_pwe sae_pwe;
+    int sae_pwe;

      /**
       * sae_pmkid_in_assoc - Whether to include PMKID in SAE Assoc Req
@@ -1415,6 +1435,7 @@ struct wpa_config {
      /**
       * mac_addr - MAC address policy default
       *
+     * Enumerations are defined in enum wpas_mac_addr_style.
       * 0 = use permanent MAC address
       * 1 = use random MAC address for each ESS connection
       * 2 = like 1, but maintain OUI (with local admin bit set)
@@ -1423,7 +1444,7 @@ struct wpa_config {
       * the per-network mac_addr parameter. Global mac_addr=1 can be 
used to
       * change this default behavior.
       */
-    enum wpas_mac_addr_style mac_addr;
+    int mac_addr;

      /**
       * rand_addr_lifetime - Lifetime of random MAC address in seconds
@@ -1433,11 +1454,12 @@ struct wpa_config {
      /**
       * preassoc_mac_addr - Pre-association MAC address policy
       *
+     * Enumerations are defined in enum wpas_mac_addr_style.
       * 0 = use permanent MAC address
       * 1 = use random MAC address
       * 2 = like 1, but maintain OUI (with local admin bit set)
       */
-    enum wpas_mac_addr_style preassoc_mac_addr;
+    int preassoc_mac_addr;

      /**
       * key_mgmt_offload - Use key management offload
@@ -1573,8 +1595,10 @@ struct wpa_config {

      /**
       * mbo_cell_capa - Cellular capabilities for MBO
+     *
+     * Enumerations are defined in enum mbo_cellular_capa.
       */
-    enum mbo_cellular_capa mbo_cell_capa;
+    int mbo_cell_capa;

      /**
       * disassoc_imminent_rssi_threshold - RSSI threshold of candidate AP
@@ -1635,11 +1659,12 @@ struct wpa_config {
      /**
       * gas_rand_mac_addr - GAS MAC address policy
       *
+     * Enumerations are defined in enum wpas_mac_addr_style.
       * 0 = use permanent MAC address
       * 1 = use random MAC address
       * 2 = like 1, but maintain OUI (with local admin bit set)
       */
-    enum wpas_mac_addr_style gas_rand_mac_addr;
+    int gas_rand_mac_addr;

      /**
       * dpp_config_processing - How to process DPP configuration
@@ -1789,13 +1814,8 @@ struct wpa_config {
  #endif /* CONFIG_PASN*/

  #ifdef CONFIG_TESTING_OPTIONS
-    enum {
-        MLD_CONNECT_BAND_PREF_AUTO = 0,
-        MLD_CONNECT_BAND_PREF_2GHZ = 1,
-        MLD_CONNECT_BAND_PREF_5GHZ = 2,
-        MLD_CONNECT_BAND_PREF_6GHZ = 3,
-        MLD_CONNECT_BAND_PREF_MAX = 4,
-    }  mld_connect_band_pref;
+    /* Enumerations are defined in enum mld_connect_band_pref. */
+    int mld_connect_band_pref;

      u8 mld_connect_bssid_pref[ETH_ALEN];

diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h
index ff045380e..b786e7f91 100644
--- a/wpa_supplicant/config_ssid.h
+++ b/wpa_supplicant/config_ssid.h
@@ -86,6 +86,9 @@ enum wpas_mac_addr_style {
   * data is included in the per-interface configuration data as an 
element of
   * the network list, struct wpa_config::ssid. Each network block in the
   * configuration is mapped to a struct wpa_ssid instance.
+ *
+ * Warning: Pay attention to using enums in the structure as treating 
enums as ints
+ * in the config parser could be dangerous if sizeof(enum) != sizeof(int).
   */
  struct wpa_ssid {
      /**
@@ -423,6 +426,8 @@ struct wpa_ssid {
      /**
       * mode - IEEE 802.11 operation mode (Infrastucture/IBSS)
       *
+     * Enumerations are defined in enum wpas_mode.
+     *
       * 0 = infrastructure (Managed) mode, i.e., associate with an AP.
       *
       * 1 = IBSS (ad-hoc, peer-to-peer)
@@ -444,7 +449,7 @@ struct wpa_ssid {
       * CCMP, but not both), and psk must also be set (either directly or
       * using ASCII passphrase).
       */
-    enum wpas_mode mode;
+    int mode;

      /**
       * pbss - Whether to use PBSS. Relevant to DMG networks only.
@@ -490,6 +495,8 @@ struct wpa_ssid {
      /**
       * ieee80211w - Whether management frame protection is enabled
       *
+     * Enumerations are defined in enum mfp_options.
+     *
       * This value is used to configure policy for management frame
       * protection (IEEE 802.11w). 0 = disabled, 1 = optional, 2 = 
required.
       * This is disabled by default unless the default value has been 
changed
@@ -499,7 +506,7 @@ struct wpa_ssid {
       * was not specified in the configuration (i.e., default behavior is
       * followed).
       */
-    enum mfp_options ieee80211w;
+    int ieee80211w;

  #ifdef CONFIG_OCV
      /**
@@ -586,7 +593,12 @@ struct wpa_ssid {

      int eht;

-    enum oper_chan_width max_oper_chwidth;
+    /**
+     * Maximum oper channel width
+     *
+     * Enumerations are defined in enum oper_chan_width.
+     */
+    int max_oper_chwidth;

      unsigned int vht_center_freq1;
      unsigned int vht_center_freq2;
@@ -605,12 +617,13 @@ struct wpa_ssid {
       * broken implementations and should be avoided when using or
       * interacting with one.
       *
+     * Enumerations are defined in enum ptk0_rekey_handling.
       * 0 = always rekey when configured/instructed
       * 1 = only rekey when the local driver is explicitly indicating 
it can
       *    perform this operation without issues
       * 2 = never allow PTK0 rekeys
       */
-    enum ptk0_rekey_handling wpa_deny_ptk0_rekey;
+    int wpa_deny_ptk0_rekey;

      /**
       * group_rekey - Group rekeying time in seconds
@@ -1008,6 +1021,7 @@ struct wpa_ssid {
      /**
       * mac_addr - MAC address policy
       *
+     * Enumerations are defined in enum wpas_mac_addr_style.
       * 0 = use permanent MAC address
       * 1 = use random MAC address for each ESS connection
       * 2 = like 1, but maintain OUI (with local admin bit set)
@@ -1017,7 +1031,7 @@ struct wpa_ssid {
       * was not specified in the configuration (i.e., default behavior is
       * followed).
       */
-    enum wpas_mac_addr_style mac_addr;
+    int mac_addr;

      /**
       * mac_value - Specific MAC address to be used
@@ -1217,13 +1231,15 @@ struct wpa_ssid {

      /**
       * sae_pk - SAE-PK mode
+     *
+     * Enumerations are defined in enum sae_pk_mode.
       * 0 = automatic SAE/SAE-PK selection based on password; enable
       * transition mode (allow SAE authentication without SAE-PK)
       * 1 = SAE-PK only (disable transition mode; allow SAE authentication
       * only with SAE-PK)
       * 2 = disable SAE-PK (allow SAE authentication only without SAE-PK)
       */
-    enum sae_pk_mode sae_pk;
+    int sae_pk;

      /**
       * was_recently_reconfigured - Whether this SSID config has been 
changed
@@ -1241,11 +1257,12 @@ struct wpa_ssid {
       * that the parameter is not set and the global sae_pwe value needs to
       * be considered.
       *
+     * Enumerations are defined in enum sae_pwe.
       * 0 = hunting-and-pecking loop only
       * 1 = hash-to-element only
       * 2 = both hunting-and-pecking loop and hash-to-element enabled
       */
-    enum sae_pwe sae_pwe;
+    int sae_pwe;

      /**
       * disable_eht - Disable EHT (IEEE 802.11be) for this network
-- 
2.44.0


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/hostap/attachments/20240227/e265ca2e/attachment-0001.sig>


More information about the Hostap mailing list