CVE-2013-4090: ACL bug in all versions of Varnish HTTP cache

[Poul-Henning Kamp]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


						Slagelse, 2013-06-11


CVE-2013-4090:  ACL bug in all versions of Varnish HTTP cache
=============================================================

Two bugs have been found in the code which compiles VCL ACLs into C code.


TL;DR
- -----

In certain cases an ACL entry will match *less* than it should.

If you use ACLs to *deny* access, some IP numbers which should
have been denied, might slip through.


Impact
- ------

These bugs only affect non-class CIDR ACL entries (ie: /xx except
/8, /16, /24 ...) when more specific ACL entries are also present.

The impact can best be explained with two examples:

        acl foo {
                "127.0.0.2";
                "127.0.0.0"/19;
	}

When the CIDR split byte matches a more specific entry, the CIDR
mask is rounded up to a byte boundary so the .../19 becomes .../24
instead, with the result that:

	127.0.0.2 matches
	127.0.0.1 matches
	127.0.1.0 does not match

If however, the CIDR split byte does not match the more specific
entry:

        acl foo {
                "127.0.0.2";
                "127.0.1.0"/19;
	}

the CIDR entry will be ignored where the split byte matches the
tested IP number, so that:

	127.0.0.2 matches
	127.0.0.1 does not match
	127.0.1.0 matches

The overall effect is always that the ACL matches less than it should.

It does not matter which byte is split by a CIDR mask.

IPv4 and IPv6 are equally impacted.

If you use the ACL to *deny* access, either by having '!' prefixes
in front of ACL entries or by the way the ACL is used, some IPs
may not be denied which should be.


Fixes
- -----

These bugs will be fixed in the Varnish 3.0.4 and subsequent releases.

Varnish 3.0.4 is due to be released this week.

For older versions of Varnish, apply the patch below.


Patch
- -----

Be aware that this patch may have white-space mangled so it might not
apply automatically, but it is simple enough to apply by hand.

diff --git a/lib/libvcl/vcc_acl.c b/lib/libvcl/vcc_acl.c
index 9c9e117..eb3bace 100644
- --- a/lib/libvcl/vcc_acl.c
+++ b/lib/libvcl/vcc_acl.c
@@ -381,7 +381,7 @@ vcc_acl_emit(const struct vcc *tl, const char *acln, int anon)
	VTAILQ_FOREACH(ae, &tl->acl, list) {
 
		/* Find how much common prefix we have */
- -		for (l = 0; l <= depth && l * 8 < ae->mask; l++) {
+		for (l = 0; l <= depth && l * 8 < ae->mask - 7; l++) {
			assert(l >= 0);
			if (ae->data[l] != at[l])
				break;
@@ -392,11 +392,11 @@ vcc_acl_emit(const struct vcc *tl, const char *acln, int anon)
		while (l <= depth) {
			Fh(tl, 0, "\t%*s}\n", -depth, "");
			depth--;
- -			oc = "else ";
		}
 
		m = ae->mask;
		m -= l * 8;
+		assert(m >= 0);
 
		/* Do whole byte compares */
		for (i = l; m >= 8; m -= 8, i++) {


Testcase
- --------

	varnishtest "acl miscompile"

	server s1 {
		rxreq
		txresp
	} -start

	varnish v1 -vcl+backend {
		acl foo {
			"127.0.0.2";
			"127.0.1"/19;
		}
		acl bar {
			"127.0.1.2";
			"127.0.1"/19;
		}
		sub vcl_deliver {
			set resp.http.ACLfoo = client.ip ~ foo;
			set resp.http.ACLbar = client.ip ~ bar;
		}
	} -start

	client c1 {
		txreq
		rxresp
		expect resp.http.aclfoo == true
		expect resp.http.aclbar == true
	} -run


*END*
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iEYEARECAAYFAlG2+V8ACgkQlftZhnGqOJNJFwCfThtKazDolUSdmD5vFvUcJk3o
hVgAn2/ZclKFsVqHdEvxBcAIzqhd8Tuj
=Pwei
-----END PGP SIGNATURE-----

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.