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
