[pkg-lighttpd] Bug#857255: lighttpd: mod_scgi: out of bounds read in scgi_demux_response

[Stefan Bühler]

Hi Helmut,

On 03/09/2017 10:06 AM, Helmut Grohne wrote:
> Package: lighttpd
> Version: 1.4.45-1
> Tags: security patch
> 
> While debugging a problem with lighttpd on behalf of my current employer
> Intenta GmbH, I found an out of bounds read.
> 
> http://sources.debian.net/src/lighttpd/1.4.45-1/src/mod_scgi.c/#L1828
> | 			for (c = hctx->response_header->ptr, cp = 0, used = buffer_string_length(hctx->response_header); used; c++, cp++, used--) {
> | 				if (*c == ':') in_header = 1;
> | 				else if (*c == '\n') {
> | 					if (in_header == 0) {
> | 						/* got a response without a response header */
> |
> | 						c = NULL;
> | 						header_end = 1;
> | 						break;
> | 					}
> |
> | 					if (eol == EOL_UNSET) eol = EOL_N;
> |
> | 					if (*(c+1) == '\n') {
> | 						header_end = 1;
> | 						hlen = cp + 2;
> | 						break;
> | 					}
> |
> | 				} else if (used > 1 && *c == '\r' && *(c+1) == '\n') {
> 
> The loop is constructed such that up to `used` bytes can be read
> starting from `c`. Thus the access to `*c` is ok. However accessing
> `*(c+1)` may be out of bounds. The condition should check for `used > 1`
> before accessing `*(c+1)`. Both the later condition checking for CR LF
> in the excerpt above and an even later condition checking for double CR
> LF do check for sufficient buffer contents. It's only this one
> occurrence that misses the check. In practise, this can result in
> lighttpd sending corrupted responses to its clients when its SCGI reads
> are chunked in a bad way (i.e. they end with '\n').

Since a buffer-API-refactoring some time ago all buffers should be
NUL-terminated.  I.e. if `*c == '\n'` is true `c` must not point to the
last character of the buffer.

If you have reason to believe otherwise please let us know :)

If you managed to trigger this in some older version we'd be interested
to know about it too - some dists are using pretty old versions.

> The following patch fixes the problem:
> --- a/src/mod_scgi.c
> +++ b/src/mod_scgi.c
> @@ -1826,7 +1826,7 @@
> 
>  					if (eol == EOL_UNSET) eol = EOL_N;
> 
> -					if (*(c+1) == '\n') {
> +					if (used > 1 && *(c+1) == '\n') {
>  						header_end = 1;
>  						hlen = cp + 2;
>  						break;

Patch looks fine though.

- Stefan