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.
This commit is contained in:
Willy Tarreau 2015-01-08 11:34:55 +01:00
parent 75abcb3106
commit 0428a146c0

View 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, which ensures that once all pending data are forwarded, the
* buffer still has global.tune.maxrewrite bytes free. The result is * buffer still has global.tune.maxrewrite bytes free. The result is
* between 0 and global.tune.maxrewrite, which is itself smaller than * 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) 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) if (chn->to_forward == CHN_INFINITE_FORWARD ||
return 0; chn->to_forward >= reserved ||
if (ret <= 0) chn->buf->o >= reserved ||
return 0; chn->to_forward + chn->buf->o >= reserved)
return ret; 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 /* Return the max number of bytes the buffer can contain so that once all the