[squid-dev] checklist->conn assertion in DestinationIp.cc

[Alex Rousskov]

Hello,

    Intercepting Squids sometimes fail with the following assertion in
ACLDestinationIP::match():

>     // Bug 3243: CVE 2009-0801
>     // Bypass of browser same-origin access control in intercepted communication
>     // To resolve this we will force DIRECT and only to the original client destination.
>     // In which case, we also need this ACL to accurately match the destination
>     if (Config.onoff.client_dst_passthru && ... intercepted ...) {
>         assert(checklist->conn() && checklist->conn()->clientConnection != NULL);
>         return ACLIP::match(checklist->conn()->clientConnection->local);
>     }

There are several reports about these failures on squid-users, including
http://lists.squid-cache.org/pipermail/squid-users/2015-May/003562.html

The assertion makes no sense to me -- why would an ACL assert that a
connection is valid? A lot of things can happen between the time the ACL
checklist was formed and the time the ACL got evaluated. This is true
for all ACLs, but should be especially obvious for slow/asynchronous
ACLs such as "dst".

Is suggest replacing the assert with an if-statement to return -1
(matching failure) when the connection is gone. Rationale: With the
connection gone, the matching result probably does not matter anymore so
there is little incentive for us to use alternative (and insecure!)
sources of destination information.

Any better ideas?


Thank you,

Alex.