http://www.squid-cache.org/bugs/show_bug.cgi?id=2526
The patch closes the ACLChecklist API against implicit allows.
I expect it to show up places which were using the behavior for their
own defaults.
Can people please check anything they can think of to check against it.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE5 or 3.0.STABLE10
Current Beta Squid 3.1.0.2
=== modified file 'src/ACLChecklist.cc'
--- src/ACLChecklist.cc 2008-10-14 09:54:26 +0000
+++ src/ACLChecklist.cc 2008-11-19 10:42:25 +0000
@@ -125,7 +125,7 @@
if (checking())
return;
- /* deny if no rules present */
+ /** Deny if no rules present. */
currentAnswer(ACCESS_DENIED);
if (callerGone()) {
@@ -133,11 +133,21 @@
return;
}
+ /** The ACL List should NEVER be NULL when calling this method.
+ * Always caller should check for NULL and handle appropriate to its needs first.
+ * We cannot select a sensible default for all callers here. */
+ if (accessList == NULL) {
+ debugs(28, DBG_CRITICAL, "SECURITY ERROR: ACL " << this << " checked with nothing to match against!!");
+ currentAnswer(ACCESS_ERROR);
+ checkCallback(currentAnswer());
+ return;
+ }
+
/* NOTE: This holds a cbdata reference to the current access_list
* entry, not the whole list.
*/
while (accessList != NULL) {
- /*
+ /** \par
* If the _acl_access is no longer valid (i.e. its been
* freed because of a reconfigure), then bail on this
* access check. For now, return ACCESS_DENIED.
@@ -158,9 +168,8 @@
}
if (finished()) {
- /*
- * We are done. Either the request
- * is allowed, denied, requires authentication.
+ /** \par
+ * Either the request is allowed, denied, requires authentication.
*/
debugs(28, 3, "ACLChecklist::check: " << this << " match found, calling back with " << currentAnswer());
cbdataReferenceDone(accessList); /* A */
@@ -181,9 +190,8 @@
cbdataReferenceDone(A);
}
- /* dropped off the end of the list */
- debugs(28, 3, "ACLChecklist::check: " << this <<
- " NO match found, returning " <<
+ /** If dropped off the end of the list return inversion of last line allow/deny action. */
+ debugs(28, 3, HERE << this << " NO match found, returning " <<
(currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED));
checkCallback(currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED);
