diff mbox

Increase buffer size and prevent write beyond buffer end

Message ID 1381488707-15762-1-git-send-email-pontus.fuchs@gmail.com
State Accepted
Commit 5079392954c80573234952772dc39dbb5d3179ca
Headers show

Commit Message

Pontus Fuchs Oct. 11, 2013, 10:51 a.m. UTC 
wpa_config_write_key_mgmt has a buffer size of 50. This is not enough
to fit the longest case. I used a network with "WPA-PSK WPA-EAP
WPA-NONE" and CONFIG_IEEE80211R=y + CONFIG_IEEE80211W=y to produce
a string longer than 50 chars. Increase the buffer size to 100 to
prevent truncated output.

Truncated output is not the only problem. If the buffer end is
reached when adding certain key mgmt types the function does not
return immediately. This leaves pos > end. When a second os_sprintf
is called the calculation of end - pos yields a large positive
number for buffer size. End result is a write beyond the buffer end.
Fix this by bailing out if buffer end is reached.

Signed-hostap: Pontus Fuchs <pontus.fuchs@gmail.com>
---
 wpa_supplicant/config.c | 64 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 17 deletions(-)

Comments

Jouni Malinen Oct. 14, 2013, 6:16 p.m. UTC | #1 
On Fri, Oct 11, 2013 at 12:51:47PM +0200, Pontus Fuchs wrote:
> wpa_config_write_key_mgmt has a buffer size of 50. This is not enough
> to fit the longest case. I used a network with "WPA-PSK WPA-EAP
> WPA-NONE" and CONFIG_IEEE80211R=y + CONFIG_IEEE80211W=y to produce
> a string longer than 50 chars. Increase the buffer size to 100 to
> prevent truncated output.
> 
> Truncated output is not the only problem. If the buffer end is
> reached when adding certain key mgmt types the function does not
> return immediately. This leaves pos > end. When a second os_sprintf
> is called the calculation of end - pos yields a large positive
> number for buffer size. End result is a write beyond the buffer end.
> Fix this by bailing out if buffer end is reached.

Thanks, applied.
diff mbox

Patch

diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index f8ebc0d..b25bd79 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -546,10 +546,10 @@  static char * wpa_config_write_key_mgmt(const struct parse_data *data,
 	char *buf, *pos, *end;
 	int ret;
 
-	pos = buf = os_zalloc(50);
+	pos = buf = os_zalloc(100);
 	if (buf == NULL)
 		return NULL;
-	end = buf + 50;
+	end = buf + 100;
 
 	if (ssid->key_mgmt & WPA_KEY_MGMT_PSK) {
 		ret = os_snprintf(pos, end - pos, "%sWPA-PSK",
@@ -602,29 +602,59 @@  static char * wpa_config_write_key_mgmt(const struct parse_data *data,
 	}
 
 #ifdef CONFIG_IEEE80211R
-	if (ssid->key_mgmt & WPA_KEY_MGMT_FT_PSK)
-		pos += os_snprintf(pos, end - pos, "%sFT-PSK",
-				   pos == buf ? "" : " ");
+	if (ssid->key_mgmt & WPA_KEY_MGMT_FT_PSK) {
+		ret = os_snprintf(pos, end - pos, "%sFT-PSK",
+				  pos == buf ? "" : " ");
+		if (ret < 0 || ret >= end - pos) {
+			end[-1] = '\0';
+			return buf;
+		}
+		pos += ret;
+	}
 
-	if (ssid->key_mgmt & WPA_KEY_MGMT_FT_IEEE8021X)
-		pos += os_snprintf(pos, end - pos, "%sFT-EAP",
-				   pos == buf ? "" : " ");
+	if (ssid->key_mgmt & WPA_KEY_MGMT_FT_IEEE8021X) {
+		ret = os_snprintf(pos, end - pos, "%sFT-EAP",
+				  pos == buf ? "" : " ");
+		if (ret < 0 || ret >= end - pos) {
+			end[-1] = '\0';
+			return buf;
+		}
+		pos += ret;
+	}
 #endif /* CONFIG_IEEE80211R */
 
 #ifdef CONFIG_IEEE80211W
-	if (ssid->key_mgmt & WPA_KEY_MGMT_PSK_SHA256)
-		pos += os_snprintf(pos, end - pos, "%sWPA-PSK-SHA256",
-				   pos == buf ? "" : " ");
+	if (ssid->key_mgmt & WPA_KEY_MGMT_PSK_SHA256) {
+		ret = os_snprintf(pos, end - pos, "%sWPA-PSK-SHA256",
+				  pos == buf ? "" : " ");
+		if (ret < 0 || ret >= end - pos) {
+			end[-1] = '\0';
+			return buf;
+		}
+		pos += ret;
+	}
 
-	if (ssid->key_mgmt & WPA_KEY_MGMT_IEEE8021X_SHA256)
-		pos += os_snprintf(pos, end - pos, "%sWPA-EAP-SHA256",
-				   pos == buf ? "" : " ");
+	if (ssid->key_mgmt & WPA_KEY_MGMT_IEEE8021X_SHA256) {
+		ret = os_snprintf(pos, end - pos, "%sWPA-EAP-SHA256",
+				  pos == buf ? "" : " ");
+		if (ret < 0 || ret >= end - pos) {
+			end[-1] = '\0';
+			return buf;
+		}
+		pos += ret;
+		}
 #endif /* CONFIG_IEEE80211W */
 
 #ifdef CONFIG_WPS
-	if (ssid->key_mgmt & WPA_KEY_MGMT_WPS)
-		pos += os_snprintf(pos, end - pos, "%sWPS",
-				   pos == buf ? "" : " ");
+	if (ssid->key_mgmt & WPA_KEY_MGMT_WPS) {
+		ret = os_snprintf(pos, end - pos, "%sWPS",
+				  pos == buf ? "" : " ");
+		if (ret < 0 || ret >= end - pos) {
+			end[-1] = '\0';
+			return buf;
+		}
+		pos += ret;
+	}
 #endif /* CONFIG_WPS */
 
 	return buf;