From d447cd596f0a9f73850229e7fa2bdd35755dc750 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Sun, 12 Apr 2015 13:20:26 -0700 Subject: [PATCH] Updates for stricter automatic memcpy bounds checking 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: * 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. * Modify most of 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. The only element not adjusted is probe_req, since doing so will generate a compile time error, and it's not obvious to me how to fix it. Signed-off-by: Nick Kralevich --- src/common/ieee802_11_defs.h | 38 +++++++++++++++++------------------ src/crypto/fips_prf_openssl.c | 16 +++++++++++---- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h index 2e51935b8..6e9c43cb2 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,7 +506,7 @@ 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 { /* only variable items: SSID, Supported rates */ @@ -518,7 +518,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 +527,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 +541,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 +561,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 +589,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 +599,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 +607,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 d69eceabf..fb03efcd4 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]);