"In order to achieve this, the function kv_extend should be modified in order to alter..." Wrong! The header (already added to desc->http_headers) gets modified correctly through the desc->http_lastheader pointer.
I'm going to stop making assumptions... I have to get my hands on a working OBSD setup and make some real test. Best regards On Wed, Aug 14, 2019 at 2:27 AM Pablo Caballero <[email protected]> wrote: > Hi guys! I've been reading about HTTP Desync Attacks lately so I took a > look to relayd's source code to check if it's likely to be exploited. > I wasn't able to do a POC (I don't have a OBSD installation at the moment) > but I think it is. > I'm going to install a OpenBSD as soon as I can in order to test the > current code and send a patch if needed but I want to tell you what I've > seen so far. > There I go: > > If a malicious user split a "Transfer-Encoding: chunked" http header in > multiple lines along with a Content-Length header it would trick relayd to > use the content length header while forwarding the > Transfer-Encoding header downstream. > The problem is in relay_read_http: > > if (strcasecmp("Transfer-Encoding", key) == 0 && > strcasecmp("chunked", value) == 0) > desc->http_chunked = 1; > > The check for key == "Transfer-Encoding" && value == chunked isn't > deferred until the full header is read and it should. > Maybe we could defer the check until we are out of the loop. > > Replacing > if (desc->http_chunked) { > /* Chunked transfer encoding */ > cre->toread = TOREAD_HTTP_CHUNK_LENGTH; > bev->readcb = relay_read_httpchunks; > } > > by doing a lookup into desc->http_headers > > In order to achieve this, the function kv_extend should be modified in > order to alter the structure pointed by the first parameter (today it does > nothing with the first parameter) > > Also, It seems like the "lookup" label along with the goto lookup sentence > could be removed. > > Payload > POST ... > ... > Transfer-Encoding: > chunked > ... > Content-Length: whatever greater than the chunk > > 10 > 1234567890123456 > POISON! > > If I'm right this payload would result in relayd honouring the > Content-Length header (breaking RFC 2616) and probably poisoning the socket > (assuming that the downstream server would honour the Transfer-Encoding > header) > > PD: I haven't digged enough to realize if relayd reuse downstream > connections among different clients so I can't say the real impact of this > issue. > > Best regards > > Pablo >
