[PATCH] Updates for stricter automatic memcpy bounds checking

Nick Kralevich nnk at google.com
Sat Apr 11 20:54:44 EDT 2015


Both Android's libc and glibc support _FORTIFY_SOURCE, a compiler
and libc feature which inserts automatic bounds checking into
common C functions such as memcpy() and strcpy(). If a buffer
overflow occurs when calling a hardened libc function, the
automatic bounds checking will safely shutdown the program and
prevent memory corruption.

Android is experimenting with _FORTIFY_SOURCE=3, a new fortify
level which enhances memcpy() to prevent overflowing an element
of a struct. Under the enhancements, code such as

  struct foo {
    char empty[0];
    char one[1];
    char a[10];
    char b[10];
  };

  int main() {
    foo myfoo;
    int n = atoi("11");
    memcpy(myfoo.a, "01234567890123456789", n);
    return 0;
  }

will cleanly crash when the memcpy() call is made.

Fixup hostap code to support the new level. Specifically:

* Modify the structures in ieee802_11_defs.h to use ISO C99 flexible
array members (https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html)
instead of a zero length array. Zero length arrays have zero length,
and any attempt to call memcpy() on such elements will always overflow.
Flexible array members have no such limitation.

* Introduce an extra element into the structure probe_req. This is
needed to keep the code compiling when an otherwise zero length
structure would be present.

* Fixup sha1_transform so it works with the enhanced bounds checking.
The old memcpy() code was attempting to write to context.h0, but that
structure element is too small and the write was extending (by design)
into h1, h2, h3, and h4. Use explicit assignments instead of
overflowing the struct element.

Signed-off-by: Nick Kralevich <nnk at google.com>
---
 src/common/ieee802_11_defs.h  | 41 +++++++++++++++++++++--------------------
 src/crypto/fips_prf_openssl.c | 16 ++++++++++++----
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
index 2e51935..98b249a 100644
--- a/src/common/ieee802_11_defs.h
+++ b/src/common/ieee802_11_defs.h
@@ -470,35 +470,35 @@ struct ieee80211_mgmt {
 			le16 auth_transaction;
 			le16 status_code;
 			/* possibly followed by Challenge text */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED auth;
 		struct {
 			le16 reason_code;
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED deauth;
 		struct {
 			le16 capab_info;
 			le16 listen_interval;
 			/* followed by SSID and Supported rates */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED assoc_req;
 		struct {
 			le16 capab_info;
 			le16 status_code;
 			le16 aid;
 			/* followed by Supported rates */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED assoc_resp, reassoc_resp;
 		struct {
 			le16 capab_info;
 			le16 listen_interval;
 			u8 current_ap[6];
 			/* followed by SSID and Supported rates */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED reassoc_req;
 		struct {
 			le16 reason_code;
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED disassoc;
 		struct {
 			u8 timestamp[8];
@@ -506,11 +506,12 @@ struct ieee80211_mgmt {
 			le16 capab_info;
 			/* followed by some of SSID, Supported rates,
 			 * FH Params, DS Params, CF Params, IBSS Params, TIM */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED beacon;
 		struct {
+			u8 unused;
 			/* only variable items: SSID, Supported rates */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED probe_req;
 		struct {
 			u8 timestamp[8];
@@ -518,7 +519,7 @@ struct ieee80211_mgmt {
 			le16 capab_info;
 			/* followed by some of SSID, Supported rates,
 			 * FH Params, DS Params, CF Params, IBSS Params */
-			u8 variable[0];
+			u8 variable[];
 		} STRUCT_PACKED probe_resp;
 		struct {
 			u8 category;
@@ -527,7 +528,7 @@ struct ieee80211_mgmt {
 					u8 action_code;
 					u8 dialog_token;
 					u8 status_code;
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED wmm_action;
 				struct{
 					u8 action_code;
@@ -541,14 +542,14 @@ struct ieee80211_mgmt {
 					u8 action;
 					u8 sta_addr[ETH_ALEN];
 					u8 target_ap_addr[ETH_ALEN];
-					u8 variable[0]; /* FT Request */
+					u8 variable[]; /* FT Request */
 				} STRUCT_PACKED ft_action_req;
 				struct {
 					u8 action;
 					u8 sta_addr[ETH_ALEN];
 					u8 target_ap_addr[ETH_ALEN];
 					le16 status_code;
-					u8 variable[0]; /* FT Request */
+					u8 variable[]; /* FT Request */
 				} STRUCT_PACKED ft_action_resp;
 				struct {
 					u8 action;
@@ -561,23 +562,23 @@ struct ieee80211_mgmt {
 				struct {
 					u8 action;
 					u8 dialogtoken;
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED wnm_sleep_req;
 				struct {
 					u8 action;
 					u8 dialogtoken;
 					le16 keydata_len;
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED wnm_sleep_resp;
 				struct {
 					u8 action;
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED public_action;
 				struct {
 					u8 action; /* 9 */
 					u8 oui[3];
 					/* Vendor-specific content */
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED vs_public_action;
 				struct {
 					u8 action; /* 7 */
@@ -589,7 +590,7 @@ struct ieee80211_mgmt {
 					 * Session Information URL (optional),
 					 * BSS Transition Candidate List
 					 * Entries */
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED bss_tm_req;
 				struct {
 					u8 action; /* 8 */
@@ -599,7 +600,7 @@ struct ieee80211_mgmt {
 					/* Target BSSID (optional),
 					 * BSS Transition Candidate List
 					 * Entries (optional) */
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED bss_tm_resp;
 				struct {
 					u8 action; /* 6 */
@@ -607,11 +608,11 @@ struct ieee80211_mgmt {
 					u8 query_reason;
 					/* BSS Transition Candidate List
 					 * Entries (optional) */
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED bss_tm_query;
 				struct {
 					u8 action; /* 15 */
-					u8 variable[0];
+					u8 variable[];
 				} STRUCT_PACKED slf_prot_action;
 			} u;
 		} STRUCT_PACKED action;
diff --git a/src/crypto/fips_prf_openssl.c b/src/crypto/fips_prf_openssl.c
index d69ecea..fb03efc 100644
--- a/src/crypto/fips_prf_openssl.c
+++ b/src/crypto/fips_prf_openssl.c
@@ -13,13 +13,21 @@
 #include "crypto.h"
 
 
-static void sha1_transform(u8 *state, const u8 data[64])
+static void sha1_transform(u32 *state, const u8 data[64])
 {
 	SHA_CTX context;
 	os_memset(&context, 0, sizeof(context));
-	os_memcpy(&context.h0, state, 5 * 4);
+	context.h0 = state[0];
+	context.h1 = state[1];
+	context.h2 = state[2];
+	context.h3 = state[3];
+	context.h4 = state[4];
 	SHA1_Transform(&context, data);
-	os_memcpy(state, &context.h0, 5 * 4);
+	state[0] = context.h0;
+	state[1] = context.h1;
+	state[2] = context.h2;
+	state[3] = context.h3;
+	state[4] = context.h4;
 }
 
 
@@ -53,7 +61,7 @@ int fips186_2_prf(const u8 *seed, size_t seed_len, u8 *x, size_t xlen)
 
 			/* w_i = G(t, XVAL) */
 			os_memcpy(_t, t, 20);
-			sha1_transform((u8 *) _t, xkey);
+			sha1_transform(_t, xkey);
 			_t[0] = host_to_be32(_t[0]);
 			_t[1] = host_to_be32(_t[1]);
 			_t[2] = host_to_be32(_t[2]);
-- 
2.2.0.rc0.207.ga3a616c



More information about the HostAP mailing list