From: Willy Tarreau <w@1wt.eu>
Date: Thu, 8 Jan 2015 10:34:55 +0000 (+0100)
Subject: BUG/MEDIUM: channel: fix possible integer overflow on reserved size computation
X-Git-Tag: v1.6-dev1~199
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0428a146c0ee776f7c89359e8aed0d755ffbad07;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: channel: fix possible integer overflow on reserved size computation

The buffer_max_len() function is subject to an integer overflow in this
calculus :

    int ret = global.tune.maxrewrite - chn->to_forward - chn->buf->o;

  - chn->to_forward may be up to 2^31 - 1
  - chn->buf->o may be up to chn->buf->size
  - global.tune.maxrewrite is by definition smaller than chn->buf->size

Thus here we can subtract (2^31 + buf->o) (highly negative) from something
slightly positive, and result in ret being larger than expected.

Fortunately in 1.5 and 1.6, this is only used by bi_avail() which itself
is used by applets which do not set high values for to_forward so this
problem does not happen there. However in 1.4 the equivalent computation
was used to limit the size of a read and can result in a read overflow
when combined with the nasty http-send-name-header feature.

This fix must be backported to 1.5 and 1.4.
---

diff --git a/include/proto/channel.h b/include/proto/channel.h
index e38e3754a1..3ec9218645 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -258,17 +258,22 @@ static inline void channel_dont_read(struct channel *chn)
  * buffer, which ensures that once all pending data are forwarded, the
  * buffer still has global.tune.maxrewrite bytes free. The result is
  * between 0 and global.tune.maxrewrite, which is itself smaller than
- * any chn->size.
+ * any chn->size. Special care is taken to avoid any possible integer
+ * overflow in the operations.
  */
 static inline int buffer_reserved(const struct channel *chn)
 {
-	int ret = global.tune.maxrewrite - chn->to_forward - chn->buf->o;
+	unsigned int reserved = global.tune.maxrewrite;
 
-	if (chn->to_forward == CHN_INFINITE_FORWARD)
-		return 0;
-	if (ret <= 0)
-		return 0;
-	return ret;
+	if (chn->to_forward == CHN_INFINITE_FORWARD ||
+	    chn->to_forward >= reserved ||
+	    chn->buf->o >= reserved ||
+	    chn->to_forward + chn->buf->o >= reserved)
+		reserved = 0;
+	else
+		reserved -= chn->to_forward + chn->buf->o;
+
+	return reserved;
 }
 
 /* Return the max number of bytes the buffer can contain so that once all the