From 2ca7b51eb6267d878aa7c85252115bc882e1d753 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sat, 23 Mar 2013 18:11:21 +0900
Subject: [PATCH] Fix the incoming last empty header value is not checked
 properly

This change fixes the bug that spdylay_frame_unpack_nv does not check
the size of header value if it is the last value in NULL separated
list.
---
 lib/spdylay_frame.c        |  8 +++++++
 tests/main.c               |  4 ++++
 tests/spdylay_frame_test.c | 47 ++++++++++++++++++++++++++++++++++++++
 tests/spdylay_frame_test.h |  2 ++
 4 files changed, 61 insertions(+)

diff --git a/lib/spdylay_frame.c b/lib/spdylay_frame.c
index 1ae8e2af..c0204eb6 100644
--- a/lib/spdylay_frame.c
+++ b/lib/spdylay_frame.c
@@ -185,6 +185,7 @@ int spdylay_frame_unpack_nv(char ***nv_ptr, spdylay_buffer *in,
     uint32_t len;
     char *name, *val;
     char *stop;
+    int multival;
     len = spdylay_frame_get_nv_len(&reader, len_size);
     if(len == 0) {
       invalid_header_block = 1;
@@ -204,6 +205,7 @@ int spdylay_frame_unpack_nv(char ***nv_ptr, spdylay_buffer *in,
     val = data;
     spdylay_buffer_reader_data(&reader, (uint8_t*)data, len);
 
+    multival = 0;
     for(stop = data+len; data != stop; ++data) {
       if(*data == '\0') {
         *idx++ = name;
@@ -212,9 +214,15 @@ int spdylay_frame_unpack_nv(char ***nv_ptr, spdylay_buffer *in,
           invalid_header_block = 1;
         }
         val = data+1;
+        multival = 1;
       }
     }
     *data = '\0';
+    /* Check last header value is empty if NULL separator was
+       found. */
+    if(multival && val == data) {
+      invalid_header_block = 1;
+    }
     ++data;
 
     *idx++ = name;
diff --git a/tests/main.c b/tests/main.c
index f0c2e6c2..d0613d57 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -216,6 +216,10 @@ int main(int argc, char* argv[])
                    test_spdylay_frame_unpack_nv_check_name_spdy2) ||
       !CU_add_test(pSuite, "frame_unpack_nv_check_name_spdy3",
                    test_spdylay_frame_unpack_nv_check_name_spdy3) ||
+      !CU_add_test(pSuite, "frame_unpack_nv_last_empty_value_spdy2",
+                   test_spdylay_frame_unpack_nv_last_empty_value_spdy2) ||
+      !CU_add_test(pSuite, "frame_unpack_nv_last_empty_value_spdy3",
+                   test_spdylay_frame_unpack_nv_last_empty_value_spdy3) ||
       !CU_add_test(pSuite, "frame_nv_set_origin",
                    test_spdylay_frame_nv_set_origin) ||
       !CU_add_test(pSuite, "stream_add_pushed_stream",
diff --git a/tests/spdylay_frame_test.c b/tests/spdylay_frame_test.c
index a03499aa..56b2d808 100644
--- a/tests/spdylay_frame_test.c
+++ b/tests/spdylay_frame_test.c
@@ -707,6 +707,8 @@ void test_spdylay_frame_nv_3to2(void)
   spdylay_frame_nv_del(nv);
 }
 
+/* This function intentionally does not merge same header field into
+   one */
 static size_t spdylay_pack_nv(uint8_t *buf, size_t buflen, const char **nv,
                               size_t len_size)
 {
@@ -790,6 +792,51 @@ void test_spdylay_frame_unpack_nv_check_name_spdy3(void)
     (spdylay_frame_get_len_size(SPDYLAY_PROTO_SPDY3));
 }
 
+static void test_spdylay_frame_unpack_nv_last_empty_value_with(size_t len_size)
+{
+  size_t nvbuflen;
+  uint8_t nvbuf[256];
+  uint8_t *nvbufptr;
+  spdylay_buffer buffer;
+  char **outnv = 0;
+  const char hdname[] = "method";
+
+  nvbufptr = nvbuf;
+  spdylay_frame_put_nv_len(nvbufptr, 1, len_size);
+  nvbufptr += len_size;
+  spdylay_frame_put_nv_len(nvbufptr, sizeof(hdname)-1, len_size);
+  nvbufptr += len_size;
+  memcpy(nvbufptr, hdname, sizeof(hdname)-1);
+  nvbufptr += sizeof(hdname)-1;
+  spdylay_frame_put_nv_len(nvbufptr, 4, len_size);
+  nvbufptr += len_size;
+  /* Copy including terminating NULL */
+  memcpy(nvbufptr, "GET", 4);
+  nvbufptr += 4;
+  nvbuflen = nvbufptr - nvbuf;
+
+  spdylay_buffer_init(&buffer, 32);
+
+  spdylay_buffer_write(&buffer, nvbuf, nvbuflen);
+  CU_ASSERT(SPDYLAY_ERR_INVALID_HEADER_BLOCK ==
+            spdylay_frame_unpack_nv(&outnv, &buffer, len_size));
+
+  spdylay_frame_nv_del(outnv);
+  spdylay_buffer_free(&buffer);
+}
+
+void test_spdylay_frame_unpack_nv_last_empty_value_spdy2(void)
+{
+  test_spdylay_frame_unpack_nv_last_empty_value_with
+    (spdylay_frame_get_len_size(SPDYLAY_PROTO_SPDY2));
+}
+
+void test_spdylay_frame_unpack_nv_last_empty_value_spdy3(void)
+{
+  test_spdylay_frame_unpack_nv_last_empty_value_with
+    (spdylay_frame_get_len_size(SPDYLAY_PROTO_SPDY3));
+}
+
 void test_spdylay_frame_nv_set_origin(void)
 {
   spdylay_origin origin;
diff --git a/tests/spdylay_frame_test.h b/tests/spdylay_frame_test.h
index 56780869..a4ee35a6 100644
--- a/tests/spdylay_frame_test.h
+++ b/tests/spdylay_frame_test.h
@@ -50,6 +50,8 @@ void test_spdylay_frame_nv_2to3(void);
 void test_spdylay_frame_nv_3to2(void);
 void test_spdylay_frame_unpack_nv_check_name_spdy2(void);
 void test_spdylay_frame_unpack_nv_check_name_spdy3(void);
+void test_spdylay_frame_unpack_nv_last_empty_value_spdy2(void);
+void test_spdylay_frame_unpack_nv_last_empty_value_spdy3(void);
 void test_spdylay_frame_nv_set_origin(void);
 
 #endif /* SPDYLAY_FRAME_TEST_H */