From 122c61926040abd0af496aa546728f089afeb16e Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sat, 18 Feb 2012 17:25:13 +0900
Subject: [PATCH] Fixed spdylay_frame_count_nv_space() bug. Check all data is
 processed in spdylay_frame_count_unpack_nv_space()

---
 lib/spdylay_frame.c        | 19 ++++++++++++-------
 lib/spdylay_frame.h        |  3 ++-
 tests/spdylay_frame_test.c | 14 ++++++++++----
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/lib/spdylay_frame.c b/lib/spdylay_frame.c
index f543467d..3e4d9f94 100644
--- a/lib/spdylay_frame.c
+++ b/lib/spdylay_frame.c
@@ -105,7 +105,7 @@ int spdylay_frame_count_unpack_nv_space
   const size_t len_size = sizeof(uint16_t);
   int i;
   if(inlen < len_size) {
-    return SPDYLAY_ERR_INVALID_ARGUMENT;
+    return SPDYLAY_ERR_INVALID_FRAME;
   }
   n = spdylay_get_uint16(in);
   off += len_size;
@@ -114,26 +114,30 @@ int spdylay_frame_count_unpack_nv_space
     int j;
     for(j = 0; j < 2; ++j) {
       if(inlen-off < len_size) {
-        return SPDYLAY_ERR_INVALID_ARGUMENT;
+        return SPDYLAY_ERR_INVALID_FRAME;
       }
       len = spdylay_get_uint16(in+off);
       off += 2;
       if(inlen-off < len) {
-        return SPDYLAY_ERR_INVALID_ARGUMENT;
+        return SPDYLAY_ERR_INVALID_FRAME;
       }
       buflen += len+1;
       off += len;
     }
-    for(off -= len, j = off+len; off != j; ++off) {
+    for(j = off, off -= len; off != j; ++off) {
       if(in[off] == '\0') {
         ++nvlen;
       }
     }
     ++nvlen;
   }
-  *nvlen_ptr = nvlen;
-  *buflen_ptr = buflen+(nvlen*2+1)*sizeof(char*);
-  return 0;
+  if(inlen == off) {
+    *nvlen_ptr = nvlen;
+    *buflen_ptr = buflen+(nvlen*2+1)*sizeof(char*);
+    return 0;
+  } else {
+    return SPDYLAY_ERR_INVALID_FRAME;
+  }
 }
 
 int spdylay_frame_unpack_nv(char ***nv_ptr, const uint8_t *in, size_t inlen)
@@ -231,6 +235,7 @@ size_t spdylay_frame_count_nv_space(char **nv)
       sum += vallen+1;
     } else {
       prev = key;
+      prevlen = keylen;
       /* SPDY NV header does not include terminating NULL byte */
       sum += keylen+vallen+4;
     }
diff --git a/lib/spdylay_frame.h b/lib/spdylay_frame.h
index b072f430..a87ca3d2 100644
--- a/lib/spdylay_frame.h
+++ b/lib/spdylay_frame.h
@@ -242,7 +242,8 @@ int spdylay_frame_unpack_settings(spdylay_settings *frame,
  * Returns number of bytes to pack name/value pairs |nv|. This
  * function expects |nv| is sorted in ascending order of key.  This
  * function can handles duplicate keys and concatenation of thier
- * values with '\0'.
+ * values with '\0'. This function returns 0 if it succeeds, or
+ * negative error code.
  */
 size_t spdylay_frame_count_nv_space(char **nv);
 
diff --git a/tests/spdylay_frame_test.c b/tests/spdylay_frame_test.c
index f73199c2..9d13b8c0 100644
--- a/tests/spdylay_frame_test.c
+++ b/tests/spdylay_frame_test.c
@@ -148,7 +148,7 @@ void test_spdylay_frame_pack_nv_duplicate_keys()
 
 void test_spdylay_frame_count_nv_space()
 {
-  CU_ASSERT(83 == spdylay_frame_count_nv_space((char**)headers));
+  CU_ASSERT(74 == spdylay_frame_count_nv_space((char**)headers));
 }
 
 void test_spdylay_frame_count_unpack_nv_space()
@@ -161,20 +161,26 @@ void test_spdylay_frame_count_unpack_nv_space()
                                                      out, inlen));
   CU_ASSERT(6 == nvlen);
   CU_ASSERT(166 == buflen);
+
+  /* Trailing garbage */
+  CU_ASSERT(SPDYLAY_ERR_INVALID_FRAME ==
+            spdylay_frame_count_unpack_nv_space(&nvlen, &buflen,
+                                                out, inlen+2));
+
   /* Change number of nv pair to a bogus value */
   temp = spdylay_get_uint16(out);
   spdylay_put_uint16be(out, temp+1);
-  CU_ASSERT(SPDYLAY_ERR_INVALID_ARGUMENT ==
+  CU_ASSERT(SPDYLAY_ERR_INVALID_FRAME ==
             spdylay_frame_count_unpack_nv_space(&nvlen, &buflen, out, inlen));
   spdylay_put_uint16be(out, temp);
 
   /* Change the length of name to a bogus value */
   temp = spdylay_get_uint16(out+2);
   spdylay_put_uint16be(out+2, temp+1);
-  CU_ASSERT(SPDYLAY_ERR_INVALID_ARGUMENT ==
+  CU_ASSERT(SPDYLAY_ERR_INVALID_FRAME ==
             spdylay_frame_count_unpack_nv_space(&nvlen, &buflen, out, inlen));
   spdylay_put_uint16be(out+2, 65535);
-  CU_ASSERT(SPDYLAY_ERR_INVALID_ARGUMENT ==
+  CU_ASSERT(SPDYLAY_ERR_INVALID_FRAME ==
             spdylay_frame_count_unpack_nv_space(&nvlen, &buflen, out, inlen));
 }