From: Emeric Brun <ebrun@haproxy.com>
Date: Wed, 29 Mar 2017 14:32:53 +0000 (+0200)
Subject: BUG/MEDIUM: peers: fix buffer overflow control in intdecode.
X-Git-Tag: v1.8-dev1~14
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=18928af3a3e3692cc2f086f8f271538436fcbe2e;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: peers: fix buffer overflow control in intdecode.

A buffer overflow could happen if an integer is badly encoded in
the data part of a msg received from a peer. It should not happen
with authenticated peers (the handshake do not use this function).

This patch makes the code of the 'intdecode' function more robust.

It also adds some comments about the intencode function.

This bug affects versions >= 1.6.
---

diff --git a/src/peers.c b/src/peers.c
index 8ffc0cb59a..0c8861f396 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -174,6 +174,12 @@ enum {
 struct peers *peers = NULL;
 static void peer_session_forceshutdown(struct appctx *appctx);
 
+/* This function encode an uint64 to 'dynamic' length format.
+   The encoded value is written at address *str, and the
+   caller must assure that size after *str is large enought.
+   At return, the *str is set at the next Byte after then
+   encoded integer. The function returns then length of the
+   encoded integer in Bytes */
 int intencode(uint64_t i, char **str) {
 	int idx = 0;
 	unsigned char *msg;
@@ -205,36 +211,35 @@ int intencode(uint64_t i, char **str) {
    *str point on the beginning of the integer to decode
    at the end of decoding *str point on the end of the
    encoded integer or to null if end is reached */
-uint64_t intdecode(char **str, char *end) {
-	uint64_t i;
-	int idx = 0;
+uint64_t intdecode(char **str, char *end)
+{
 	unsigned char *msg;
+	uint64_t i;
+	int shift;
 
 	if (!*str)
 		return 0;
 
 	msg = (unsigned char *)*str;
-	if (msg >= (unsigned char *)end) {
-		*str = NULL;
-		return 0;
+	if (msg >= (unsigned char *)end)
+		goto fail;
+
+	i = *(msg++);
+	if (i >= 240) {
+		shift = 4;
+		do {
+			if (msg >= (unsigned char *)end)
+				goto fail;
+			i += (uint64_t)*msg << shift;
+			shift += 7;
+		} while (*(msg++) >= 128);
 	}
+        *str = (char *)msg;
+        return i;
 
-	if (msg[idx] < 240) {
-		*str = (char *)&msg[idx+1];
-		return msg[idx];
-	}
-	i = msg[idx];
-	do {
-		idx++;
-		if (msg >= (unsigned char *)end) {
-			*str = NULL;
-			return 0;
-		}
-		i += (uint64_t)msg[idx] <<  (4 + 7*(idx-1));
-	}
-	while (msg[idx] >= 128);
-	*str = (char *)&msg[idx+1];
-	return i;
+  fail:
+        *str = NULL;
+        return 0;
 }
 
 /* Set the stick-table UPDATE message type byte at <msg_type> address,