nghttp2_pack_settings_payload: added a buffer size argument

To make it less likely that a user gets a buffer overflow if a
too small buffer is used.

lib/includes/nghttp2/nghttp2.h

@@ -254,6 +254,10 @@ typedef enum {
    * Flow control error
    */
   NGHTTP2_ERR_FLOW_CONTROL = -524,
+  /**
+   * Insufficient buffer size given to function.
+   */
+  NGHTTP2_ERR_INSUFF_BUFSIZE = -525,
   /**
    * The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is
    * under unexpected condition and cannot process any further data
@@ -1502,13 +1506,13 @@ int nghttp2_session_upgrade(nghttp2_session *session,
 /**
  * @function
  *
- * Serializes the SETTINGS values |iv| in the |buf|. The number of
+ * Serializes the SETTINGS values |iv| in the |buf|. The size of the |buf| is
- * entry pointed by |iv| array is given by the |niv|. This function
+ * specified by |buflen|. The number of entries in the |iv| array is given by
- * may reorder the pointers in |iv|. The |buf| must have enough region
+ * |niv|. This function may reorder the pointers in |iv|. The required space
- * to hold serialized data. The required space for the |niv| entries
+ * in |buf| for the |niv| entries is ``8*niv`` bytes and if the given buffer
- * are ``8*niv`` bytes. This function is used mainly for creating
+ * is too small, an error is returned. This function is used mainly for
- * SETTINGS payload to be sent with ``HTTP2-Settings`` header field in
+ * creating a SETTINGS payload to be sent with the ``HTTP2-Settings`` header
- * HTTP Upgrade request. The data written in |buf| is NOT
+ * field in an HTTP Upgrade request. The data written in |buf| is NOT
  * base64url encoded and the application is responsible for encoding.
  *
  * This function returns the number of bytes written in |buf|, or one
@@ -1516,8 +1520,12 @@ int nghttp2_session_upgrade(nghttp2_session *session,
  *
  * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
  *     The |iv| contains duplicate settings ID or invalid value.
+ *
+ * :enum:`NGHTTP2_ERR_INSUFF_BUFSIZE`
+ *     The provided |buflen| size is too small to hold the output.
  */
 ssize_t nghttp2_pack_settings_payload(uint8_t *buf,
+                                      size_t buflen,
                                       nghttp2_settings_entry *iv, size_t niv);
 
 /**

lib/nghttp2_submit.c

@@ -1,7 +1,7 @@
 /*
  * nghttp2 - HTTP/2.0 C Library
  *
- * Copyright (c) 2012 Tatsuhiro Tsujikawa
+ * Copyright (c) 2012, 2013 Tatsuhiro Tsujikawa
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files (the
@@ -348,6 +348,7 @@ int nghttp2_submit_data(nghttp2_session *session, uint8_t flags,
 }
 
 ssize_t nghttp2_pack_settings_payload(uint8_t *buf,
+                                      size_t buflen,
                                       nghttp2_settings_entry *iv, size_t niv)
 {
   /* Assume that current flow_control_option is 0 (which means that
@@ -355,5 +356,9 @@ ssize_t nghttp2_pack_settings_payload(uint8_t *buf,
   if(!nghttp2_iv_check(iv, niv, 0)) {
     return NGHTTP2_ERR_INVALID_ARGUMENT;
   }
+
+  if(buflen < (niv * 8))
+    return NGHTTP2_ERR_INSUFF_BUFSIZE;
+
   return nghttp2_frame_pack_settings_payload(buf, iv, niv);
 }

tests/nghttp2_session_test.c

@@ -1544,7 +1544,9 @@ void test_nghttp2_session_upgrade(void)
   iv[0].value = 1;
   iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
   iv[1].value = 4095;
-  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload, iv, 2);
+  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload,
+                                                      sizeof(settings_payload),
+                                                      iv, 2);
 
   /* Check client side */
   nghttp2_session_client_new(&session, &callbacks, NULL);
@@ -1594,7 +1596,9 @@ void test_nghttp2_session_upgrade(void)
   /* Check required settings */
   iv[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS;
   iv[0].value = 1;
-  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload, iv, 1);
+  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload,
+                                                      sizeof(settings_payload),
+                                                      iv, 1);
 
   nghttp2_session_client_new(&session, &callbacks, NULL);
   CU_ASSERT(NGHTTP2_ERR_PROTO ==
@@ -1604,7 +1608,9 @@ void test_nghttp2_session_upgrade(void)
 
   iv[0].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
   iv[0].value = 4095;
-  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload, iv, 1);
+  settings_payloadlen = nghttp2_pack_settings_payload(settings_payload,
+                                                      sizeof(settings_payload),
+                                                      iv, 1);
 
   nghttp2_session_client_new(&session, &callbacks, NULL);
   CU_ASSERT(NGHTTP2_ERR_PROTO ==
@@ -3278,7 +3284,7 @@ void test_nghttp2_pack_settings_payload(void)
 {
   nghttp2_settings_entry iv[2];
   uint8_t buf[64];
-  size_t len;
+  ssize_t len;
   nghttp2_settings_entry *resiv;
   size_t resniv;
 
@@ -3287,7 +3293,7 @@ void test_nghttp2_pack_settings_payload(void)
   iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
   iv[1].value = 4095;
 
-  len = nghttp2_pack_settings_payload(buf, iv, 2);
+  len = nghttp2_pack_settings_payload(buf, sizeof(buf), iv, 2);
   CU_ASSERT(16 == len);
   CU_ASSERT(0 == nghttp2_frame_unpack_settings_payload(&resiv, &resniv,
                                                        buf, len));
@@ -3298,4 +3304,7 @@ void test_nghttp2_pack_settings_payload(void)
   CU_ASSERT(4095 == resiv[1].value);
 
   free(resiv);
+
+  len = nghttp2_pack_settings_payload(buf, 15 /* too small */, iv, 2);
+  CU_ASSERT(NGHTTP2_ERR_INSUFF_BUFSIZE == len);
 }