squid-1702

Version:

2.6.STABLE1

Bug link:

http://bugs.squid-cache.org/show_bug.cgi?id=1702

How it is diagnosed (reproduced or source analysis)?

We reproduced the failure.

Add the following 2 lines into the configuration file: etc/squid.conf

acl abcdefghijklmnopqrstuvwxyzabcdefghijklmn src VALIDIP

http_access allow abcdefghijklmnopqrstuvwxyzabcdefghijklmn

Use a valid IP address for “VALIDIP”

Then start squid:

$ squid -NCd10

Symptom:

A long ‘acl name’ in configuration file would cause squid to fail with ambiguous log message:

FATAL: Bungled squid.conf line 2368: http_access allow
abcdefghijklmnopqrstuvwxyzabcdef

Root cause:

Squid forgot to check an input that is too long.

defines.h:

#define ACL_NAME_SZ 32 // So the length limitation of ACL name is 32.

Here is the patch:

--- acl.c        2006-07-28 10:04:39.000000000 +0530

+++ acl.c_        2006-07-28 10:01:23.000000000 +0530

aclParseAclList

@@ -1268,6 +1268,10 @@

            t++;

        }

        debug(28, 3) ("aclParseAccessLine: looking for ACL name '%s'\n", t);

+        if(strlen(t) > 31){

+        debug(28, 0) ("aclParseAclLine: ACL name '%s' too big \n", t);

+        self_destruct();

+        }

        a = aclFindByName(t);

        if (a == NULL) {

            debug(28, 0) ("ACL name '%s' not defined!\n", t);

static acl *

aclFindByName(const char *name)

{

   acl *a;

   /* aclFindByName is searching a list of available ACL names. It returns NULL when cannot find. This belongs to generalized default-switch pattern, when the program encounters unexpected state. */

   for (a = Config.aclList; a; a = a->next)

      if (!strcasecmp(a->name, name))

         return a;

   /* In the failure case: it returned NULL here, because squid cannot cannot find the ‘name’ in the aclList. */

   /* Errlog and developers can log here, since semantically it is a ‘default’ case in a switch statement, just squid is using a loop to implement to switch logic. */

   return NULL;

}

Is there Error Message?

YES.

Can we anticipate the Error:

Yes. This is a generalized default-switch pattern. The code above (aclFindByName) uses this the following pattern:

foreach (acceptable element) {

 if (target == element)

   return element;

}

/* default, log! */

return cannot_find;