]> 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)
committerSquid Anubis <squid-anubis@squid-cache.org>
Sun, 12 Nov 2023 04:21:04 +0000 (04:21 +0000)
commitb850f8b2161635fba683e9f55e8a2f068937174a
treeed791e4b7556f863228c292ce2ba9b5f0b490f15
parent908daabd679f0eb7c325eda9f6c15a10441b40f3
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