]> git.ipfire.org Git - thirdparty/squid.git/commit
Just close after a write(2) response sending error (#1582)
authorAlex Rousskov <rousskov@measurement-factory.com>
Sun, 12 Nov 2023 00:44:19 +0000 (00:44 +0000)
committerAmos Jeffries <yadij@users.noreply.github.com>
Tue, 28 Nov 2023 14:03:29 +0000 (03:03 +1300)
commit61c9e763b4baa69facbd502aacb991f2bfa31293
treeb03b2200671e173c76be7b2afb65b5280a8cddb0
parenta6e005ec07a419226dfc93515d9d6340684db35c
Just close after a write(2) response sending error (#1582)

    FATAL: assertion failed: Http1Server.cc:322: "rep"

2015 commit 21cd322 started to continue ClientStream processing after
socket write(2) failures. In most cases, the code still "worked". For
example, initiateClose() would close the client-Squid connection, and
connStateClosed() would be called before Store has a chance to deliver
response body data requested by pullData() in writeComplete().

However, that response body data could sometimes reach Server, and
handleReply() would assert because startOfOutput() says that we have not
written the headers, but ClientStream state (i.e. a nil `rep` parameter)
says that we have. These assertion can be triggered by disabling
initiateClose(), and they can probably be triggered by traffic as well.

Now, after a Comm::Write() error, we terminateAll() client transactions
on the failed connection[^1] and do not call afterClientWrite() that is
not equipped to handle I/O errors and would continue ClientStream
processing if called.

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/stream-assert.html
where it was filed as "Implicit Assertion in Stream Handling".

[^1]: We terminateAll() instead of potentially postponing closure with
initiateClose() because the failed client-Squid connection most likely
cannot be salvaged for, say, reading the remainder of the request body.
src/servers/Server.cc