[master] 7fd3aa3 Don't let the form of the argument to hash_data() leak into the production of the hash-key.

[Poul-Henning Kamp]

commit 7fd3aa3f8fdf97185af22ce9e71c338933ee1cc0
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed May 1 13:13:16 2013 +0000

    Don't let the form of the argument to hash_data() leak into the
    production of the hash-key.
    
    Fixes	#1296

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index eab4907..3f4fb79 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -155,20 +155,15 @@ HSH_DeleteObjHead(struct dstat *ds, struct objhead *oh)
 }
 
 void
-HSH_AddString(struct req *req, const char *str)
+HSH_AddString(const struct req *req, const char *str)
 {
-	int l;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-	if (str == NULL)
-		str = "";
-	l = strlen(str);
-
 	AN(req->sha256ctx);
-	SHA256_Update(req->sha256ctx, str, l);
-	SHA256_Update(req->sha256ctx, "#", 1);
-
-	VSLb(req->vsl, SLT_Hash, "%s", str);
+	if (str != NULL)
+		SHA256_Update(req->sha256ctx, str, strlen(str));
+	else
+		SHA256_Update(req->sha256ctx, &str, sizeof str);
 }
 
 /*---------------------------------------------------------------------
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index a298ad7..8d0ee46 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -257,7 +257,7 @@ VRT_handling(const struct vrt_ctx *ctx, unsigned hand)
 }
 
 /*--------------------------------------------------------------------
- * Add an element to the array/list of hash bits.
+ * Feed data into the hash calculation
  */
 
 void
@@ -275,7 +275,13 @@ VRT_hashdata(const struct vrt_ctx *ctx, const char *str, ...)
 		if (p == vrt_magic_string_end)
 			break;
 		HSH_AddString(ctx->req, p);
+		VSLb(ctx->vsl, SLT_Hash, "%s", str);
 	}
+	/*
+	 * Add a 'field-separator' to make it more difficult to
+	 * manipulate the hash.
+	 */
+	HSH_AddString(ctx->req, NULL);
 }
 
 /*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index 8a3d08b..3c36b4d 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -69,7 +69,7 @@ enum lookup_e HSH_Lookup(struct req *, struct objcore **, struct objcore **,
 void HSH_Ref(struct objcore *o);
 void HSH_Drop(struct worker *, struct object **);
 void HSH_Init(const struct hash_slinger *slinger);
-void HSH_AddString(struct req *, const char *str);
+void HSH_AddString(const struct req *, const char *str);
 void HSH_Insert(struct worker *, const void *hash, struct objcore *);
 void HSH_Purge(struct req *, struct objhead *, double ttl, double grace);
 void HSH_config(const char *h_arg);
diff --git a/bin/varnishtest/tests/r01296.vtc b/bin/varnishtest/tests/r01296.vtc
new file mode 100644
index 0000000..f9f8de4
--- /dev/null
+++ b/bin/varnishtest/tests/r01296.vtc
@@ -0,0 +1,30 @@
+varnishtest "hash key depends on argument form to hash_data()"
+
+server s1 {
+	rxreq
+	txresp -hdr "OK: yes"
+	rxreq
+	txresp -hdr "OK: no"
+} -start
+
+varnish v1 -vcl+backend {
+
+	sub vcl_hash {
+		if (req.http.foo == "1") {
+			hash_data("123");
+		} else {
+			hash_data("1" + req.http.foo + "3");
+		}
+		return (hash);
+	}
+} -start
+
+client c1 {
+	txreq -hdr "foo: 1"
+	rxresp
+	expect resp.http.ok == "yes"
+
+	txreq -hdr "foo: 2"
+	rxresp
+	expect resp.http.ok == "yes"
+} -run