]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: channel: fix possible integer overflow on reserved size computation
authorWilly Tarreau <w@1wt.eu>
Thu, 8 Jan 2015 10:34:55 +0000 (11:34 +0100)
committerWilly Tarreau <w@1wt.eu>
Wed, 14 Jan 2015 11:04:34 +0000 (12:04 +0100)
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.

include/proto/channel.h

index e38e3754a1dcc2832d7c61540277d37706e8322b..3ec92186450122be1bc89c6487d84bf1bed34a19 100644 (file)
@@ -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