[EMAIL PROTECTED]: I'm CCing you because the bug was introduced in the last security update (DSA-809-2) for Squid on Debian Woody. It would be helpful if a security update could be issued without this bug; there is a patch at the end of this message.
Credit for the discovery of the cause of this bug and providing a fix should go to "Kosa Attila" <[EMAIL PROTECTED]> (see bug #332251) I'm just (a) explaining how to reproduce the problem, (b) providing a more easily applied patch and (c) bringing it to more people's attention, since that fix seems to have been ignored. Squid -woody10 introduces a flaw which causes the FTP connections through Squid to cause squid to abort with an assertion failure when the data channel completes, eg: squid[20250]: assertion failed: comm.c:636: "F->flags.open" squid[20235]: Squid Parent: child process 20250 exited due to signal 6 This is reported as bug #331714 (mine), and #332251. The flaw is easily demonstrated by starting a FTP connection through the Squid proxy (eg, ftp_proxy=http://localhost:3128/ lynx ftp://ftp.debian.org/) and trying to download a file via FTP. Directory listings are okay (there is separate code to handle those properly), but any attempt to download a file will result in squid failing on the assertion. The cause is ftpDataRead() (src/ftp.c:872) calling commSetSelect() (at src/ftp.c:942) irrespective of whether or not the the data connection was completed, and hence ftpReadComplete() was called to close the data connection. commSetSelect() (src/comm.c:632) checks that the connection it is being told to watch is actually open with an assertion (at src/comm.c:636), which fails when called on a connection that has been closed. -woody10 (with Debian patches) contains the source: (-woody10 src/ftp.c: lines 938 to 946) -=- cut here -=- if (ftpState->size > 0 && mem->inmem_hi >= ftpState->size + mem->reply->hdr_sz) ftpReadComplete(ftpState); else storeBufferFlush(entry); commSetSelect(fd, COMM_SELECT_READ, ftpDataRead, data, Config.Timeout.read); -=- cut here -=- Despite the optimistic indenting, in C what this means is that if the file transfer completed, close the socket and then try to set up a call back on that closed socket. The flaw is obvious when comparing it with the upstream 2.4.6 source: (upstream 2.4.6 src/ftp.c: lines 924 to 931) -=- cut here -=- if (ftpState->size > 0 && mem->inmem_hi >= ftpState->size + mem->reply->hdr_sz) ftpReadComplete(ftpState); else commSetSelect(fd, COMM_SELECT_READ, ftpDataRead, data, Config.Timeout.read); -=- cut here -=- The line "storeBufferFlush(entry);" has been introduced in -woody10, which means that is reserved for the else-path rather than the commSetSelect() being reserved for the else-path (ie, connection not finished). The correct fix, as discovered by "Kosa Attila" <[EMAIL PROTECTED]>, is to enclose the else-path in braces, so that both the storeBufferFlush() and the commSetSelect() are reserved for the else-path. I have tested this on my Woody test server, and without it ftp data connections always cause a crash, and with the fix ftp connections are fine; I plan to put it into production tonight. A more easily applied patch is enclosed below. 2.5.9-10sarge2 in Debian Sarge does not suffer from ths issue because 2.5.9 has a rewritten ftpDataRead() function that has been restructured to avoid this sort of issue. Ewen -=- cut here -=- diff -ur squid-2.4.6/debian/changelog squid-2.4.6-ftpfix/debian/changelog --- squid-2.4.6/debian/changelog Tue Nov 1 13:09:43 2005 +++ squid-2.4.6-ftpfix/debian/changelog Tue Nov 1 13:03:58 2005 @@ -1,3 +1,11 @@ +squid (2.4.6-2woody10.ftpfix) oldstable-security; urgency=high + + * Added fix for broken ftpDataRead() handling (in ftp.c), which was broken + by 2.4.6-2woody10 patches. Suggested by "Kosa Attila" + <[EMAIL PROTECTED]> in #332251 + + -- Ewen McNeill <[EMAIL PROTECTED]> Tue, 1 Nov 2005 13:02:41 +1300 + squid (2.4.6-2woody10) oldstable-security; urgency=high * Upload to oldstable-security because of security issues diff -ur squid-2.4.6/src/ftp.c squid-2.4.6-ftpfix/src/ftp.c --- squid-2.4.6/src/ftp.c Tue Nov 1 13:09:43 2005 +++ squid-2.4.6-ftpfix/src/ftp.c Tue Nov 1 13:00:17 2005 @@ -937,13 +937,14 @@ } if (ftpState->size > 0 && mem->inmem_hi >= ftpState->size + mem->reply->hdr_sz) ftpReadComplete(ftpState); - else - storeBufferFlush(entry); + else { + storeBufferFlush(entry); commSetSelect(fd, COMM_SELECT_READ, ftpDataRead, data, Config.Timeout.read); + } } } -=- cut here -=- -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
