CVE-2020-11080: Improper Neutralization (CWE-707)
value: In nghttp2 before version 1.41.0, the overly large HTTP/2 SETTINGS frame payload causes denial of service. The proof of concept attack involves a malicious client constructing a SETTINGS frame with a length of 14,400 bytes (2400 individual settings entries) over and over again. The attack causes the CPU to spike at 100%. nghttp2 v1.41.0 fixes this vulnerability. There is a workaround to this vulnerability. Implement nghttp2_on_frame_recv_callback callback, and if received frame is SETTINGS frame and the number of settings entries are large (e.g., > 32), then drop the connection.
CWE description: The product does not ensure or incorrectly ensures that structured messages or data are well-formed and that certain security properties are met before being read from an upstream component or sent to a downstream component.
CVSS v3 base score: 7.5
Published: 2020-06-03T23:15Z
Repository: https://github.com/nghttp2/nghttp2
Language: C
Files changed: doc/CMakeLists.txt, doc/Makefile.am, lib/includes/nghttp2/nghttp2.h, lib/nghttp2_helper.c, lib/nghttp2_option.c, lib/nghttp2_option.h, lib/nghttp2_session.c, lib/nghttp2_session.h, tests/main.c, tests/nghttp2_session_test.c, tests/nghttp2_session_test.h
Commit date: 2020-05-05 11:55:32 -0700
Commit message: Implement max settings option

Diff: diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt
index 34c02792..f3aec84d 100644
--- a/doc/CMakeLists.txt
+++ b/doc/CMakeLists.txt
@@ -40,10 +40,11 @@ set(APIDOCS
   nghttp2_option_set_no_auto_window_update.rst
   nghttp2_option_set_no_http_messaging.rst
   nghttp2_option_set_no_recv_client_magic.rst
   nghttp2_option_set_peer_max_concurrent_streams.rst
   nghttp2_option_set_user_recv_extension_type.rst
+  nghttp2_option_set_max_settings.rst
   nghttp2_pack_settings_payload.rst
   nghttp2_priority_spec_check_default.rst
   nghttp2_priority_spec_default_init.rst
   nghttp2_priority_spec_init.rst
   nghttp2_rcbuf_decref.rst
diff --git a/doc/Makefile.am b/doc/Makefile.am
index 4d73cef5..f073bfa4 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -67,10 +67,11 @@ APIDOCS= \
 	nghttp2_option_set_no_http_messaging.rst \
 	nghttp2_option_set_no_recv_client_magic.rst \
 	nghttp2_option_set_peer_max_concurrent_streams.rst \
 	nghttp2_option_set_user_recv_extension_type.rst \
 	nghttp2_option_set_max_outbound_ack.rst \
+	nghttp2_option_set_max_settings.rst \
 	nghttp2_pack_settings_payload.rst \
 	nghttp2_priority_spec_check_default.rst \
 	nghttp2_priority_spec_default_init.rst \
 	nghttp2_priority_spec_init.rst \
 	nghttp2_rcbuf_decref.rst \
diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h
index e3aeb9fe..9be6eea5 100644
--- a/lib/includes/nghttp2/nghttp2.h
+++ b/lib/includes/nghttp2/nghttp2.h
@@ -226,10 +226,17 @@ typedef struct {
  *
  * The length of :macro:`NGHTTP2_CLIENT_MAGIC`.
  */
 #define NGHTTP2_CLIENT_MAGIC_LEN 24
 
+/**
+ * @macro
+ *
+ * The default max number of settings per SETTINGS frame
+ */
+#define NGHTTP2_DEFAULT_MAX_SETTINGS 32
+
 /**
  * @enum
  *
  * Error codes used in this library.  The code range is [-999, -500],
  * inclusive. The following values are defined:
@@ -396,10 +403,15 @@ typedef enum {
   /**
    * When a local endpoint expects to receive SETTINGS frame, it
    * receives an other type of frame.
    */
   NGHTTP2_ERR_SETTINGS_EXPECTED = -536,
+  /**
+   * When a local endpoint receives too many settings entries
+   * in a single SETTINGS frame.
+   */
+  NGHTTP2_ERR_TOO_MANY_SETTINGS = -537,
   /**
    * The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is
    * under unexpected condition and processing was terminated (e.g.,
    * out of memory).  If application receives this error code, it must
    * stop using that :type:`nghttp2_session` object and only allowed
@@ -2657,10 +2669,21 @@ NGHTTP2_EXTERN void nghttp2_option_set_no_closed_streams(nghttp2_option *option,
  * misbehaving and session will be closed.  The default value is 1000.
  */
 NGHTTP2_EXTERN void nghttp2_option_set_max_outbound_ack(nghttp2_option *option,
                                                         size_t val);
 
+/**
+ * @function
+ *
+ * This function sets the maximum number of SETTINGS entries per
+ * SETTINGS frame that will be accepted. If more than those entries
+ * are received, the peer is considered to be misbehaving and session
+ * will be closed. The default value is 32.
+ */
+NGHTTP2_EXTERN void nghttp2_option_set_max_settings(nghttp2_option *option,
+                                                    size_t val);
+
 /**
  * @function
  *
  * Initializes |*session_ptr| for client use.  The all members of
  * |callbacks| are copied to |*session_ptr|.  Therefore |*session_ptr|
diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c
index 91136a61..0bd54147 100644
--- a/lib/nghttp2_helper.c
+++ b/lib/nghttp2_helper.c
@@ -332,10 +332,12 @@ const char *nghttp2_strerror(int error_code) {
   case NGHTTP2_ERR_BAD_CLIENT_MAGIC:
     return "Received bad client magic byte string";
   case NGHTTP2_ERR_FLOODED:
     return "Flooding was detected in this HTTP/2 session, and it must be "
            "closed";
+  case NGHTTP2_ERR_TOO_MANY_SETTINGS:
+    return "SETTINGS frame contained more than the maximum allowed entries";
   default:
     return "Unknown error code";
   }
 }
 
diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c
index e53f22d3..34348e66 100644
--- a/lib/nghttp2_option.c
+++ b/lib/nghttp2_option.c
@@ -119,5 +119,10 @@ void nghttp2_option_set_no_closed_streams(nghttp2_option *option, int val) {
 
 void nghttp2_option_set_max_outbound_ack(nghttp2_option *option, size_t val) {
   option->opt_set_mask |= NGHTTP2_OPT_MAX_OUTBOUND_ACK;
   option->max_outbound_ack = val;
 }
+
+void nghttp2_option_set_max_settings(nghttp2_option *option, size_t val) {
+  option->opt_set_mask |= NGHTTP2_OPT_MAX_SETTINGS;
+  option->max_settings = val;
+}
diff --git a/lib/nghttp2_option.h b/lib/nghttp2_option.h
index 1f740aaa..939729fd 100644
--- a/lib/nghttp2_option.h
+++ b/lib/nghttp2_option.h
@@ -65,10 +65,11 @@ typedef enum {
   NGHTTP2_OPT_BUILTIN_RECV_EXT_TYPES = 1 << 7,
   NGHTTP2_OPT_MAX_SEND_HEADER_BLOCK_LENGTH = 1 << 8,
   NGHTTP2_OPT_MAX_DEFLATE_DYNAMIC_TABLE_SIZE = 1 << 9,
   NGHTTP2_OPT_NO_CLOSED_STREAMS = 1 << 10,
   NGHTTP2_OPT_MAX_OUTBOUND_ACK = 1 << 11,
+  NGHTTP2_OPT_MAX_SETTINGS = 1 << 12,
 } nghttp2_option_flag;
 
 /**
  * Struct to store option values for nghttp2_session.
  */
@@ -83,10 +84,14 @@ struct nghttp2_option {
   size_t max_deflate_dynamic_table_size;
   /**
    * NGHTTP2_OPT_MAX_OUTBOUND_ACK
    */
   size_t max_outbound_ack;
+  /**
+   * NGHTTP2_OPT_MAX_SETTINGS
+   */
+  size_t max_settings;
   /**
    * Bitwise OR of nghttp2_option_flag to determine that which fields
    * are specified.
    */
   uint32_t opt_set_mask;
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
index 563ccd7d..415e3477 100644
--- a/lib/nghttp2_session.c
+++ b/lib/nghttp2_session.c
@@ -456,10 +456,11 @@ static int session_new(nghttp2_session **session_ptr,
   /* Limit max outgoing concurrent streams to sensible value */
   (*session_ptr)->remote_settings.max_concurrent_streams = 100;
 
   (*session_ptr)->max_send_header_block_length = NGHTTP2_MAX_HEADERSLEN;
   (*session_ptr)->max_outbound_ack = NGHTTP2_DEFAULT_MAX_OBQ_FLOOD_ITEM;
+  (*session_ptr)->max_settings = NGHTTP2_DEFAULT_MAX_SETTINGS;
 
   if (option) {
     if ((option->opt_set_mask & NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE) &&
         option->no_auto_window_update) {
 
@@ -519,10 +520,15 @@ static int session_new(nghttp2_session **session_ptr,
     }
 
     if (option->opt_set_mask & NGHTTP2_OPT_MAX_OUTBOUND_ACK) {
       (*session_ptr)->max_outbound_ack = option->max_outbound_ack;
     }
+
+    if ((option->opt_set_mask & NGHTTP2_OPT_MAX_SETTINGS) &&
+        option->max_settings) {
+      (*session_ptr)->max_settings = option->max_settings;
+    }
   }
 
   rv = nghttp2_hd_deflate_init2(&(*session_ptr)->hd_deflater,
                                 max_deflate_dynamic_table_size, mem);
   if (rv != 0) {
@@ -5655,10 +5661,20 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
           /* We allocate iv with additional one entry, to store the
              minimum header table size. */
           iframe->max_niv =
               iframe->frame.hd.length / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH + 1;
 
+          if (iframe->max_niv - 1 > session->max_settings) {
+            rv = nghttp2_session_terminate_session_with_reason(
+                session, NGHTTP2_ENHANCE_YOUR_CALM,
+                "SETTINGS: too many setting entries");
+            if (nghttp2_is_fatal(rv)) {
+              return rv;
+            }
+            return (ssize_t)inlen;
+          }
+
           iframe->iv = nghttp2_mem_malloc(mem, sizeof(nghttp2_settings_entry) *
                                                    iframe->max_niv);
 
           if (!iframe->iv) {
             return NGHTTP2_ERR_NOMEM;
@@ -7423,10 +7439,15 @@ static int nghttp2_session_upgrade_internal(nghttp2_session *session,
     return NGHTTP2_ERR_PROTO;
   }
   if (settings_payloadlen % NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH) {
     return NGHTTP2_ERR_INVALID_ARGUMENT;
   }
+  /* SETTINGS frame contains too many settings */
+  if (settings_payloadlen / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH
+        > session->max_settings) {
+    return NGHTTP2_ERR_TOO_MANY_SETTINGS;
+  }
   rv = nghttp2_frame_unpack_settings_payload2(&iv, &niv, settings_payload,
                                               settings_payloadlen, mem);
   if (rv != 0) {
     return rv;
   }
diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h
index d2082731..07bfbb6c 100644
--- a/lib/nghttp2_session.h
+++ b/lib/nghttp2_session.h
@@ -265,10 +265,12 @@ struct nghttp2_session {
      outbound queue. */
   size_t max_outbound_ack;
   /* The maximum length of header block to send.  Calculated by the
      same way as nghttp2_hd_deflate_bound() does. */
   size_t max_send_header_block_length;
+  /* The maximum number of settings accepted per SETTINGS frame. */
+  size_t max_settings;
   /* Next Stream ID. Made unsigned int to detect >= (1 << 31). */
   uint32_t next_stream_id;
   /* The last stream ID this session initiated.  For client session,
      this is the last stream ID it has sent.  For server session, it
      is the last promised stream ID sent in PUSH_PROMISE. */
diff --git a/tests/main.c b/tests/main.c
index 41e0b03e..67eb4a1c 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -315,10 +315,12 @@ int main() {
                    test_nghttp2_session_repeated_priority_submission) ||
       !CU_add_test(pSuite, "session_set_local_window_size",
                    test_nghttp2_session_set_local_window_size) ||
       !CU_add_test(pSuite, "session_cancel_from_before_frame_send",
                    test_nghttp2_session_cancel_from_before_frame_send) ||
+      !CU_add_test(pSuite, "session_too_many_settings",
+                   test_nghttp2_session_too_many_settings) ||
       !CU_add_test(pSuite, "session_removed_closed_stream",
                    test_nghttp2_session_removed_closed_stream) ||
       !CU_add_test(pSuite, "session_pause_data",
                    test_nghttp2_session_pause_data) ||
       !CU_add_test(pSuite, "session_no_closed_streams",
diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
index 6eb8e244..33ee3ad8 100644
--- a/tests/nghttp2_session_test.c
+++ b/tests/nghttp2_session_test.c
@@ -10612,10 +10612,71 @@ void test_nghttp2_session_cancel_from_before_frame_send(void) {
   CU_ASSERT(NULL == stream);
 
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_session_too_many_settings(void) {
+  nghttp2_session *session;
+  nghttp2_option *option;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_frame frame;
+  nghttp2_bufs bufs;
+  nghttp2_buf *buf;
+  ssize_t rv;
+  my_user_data ud;
+  nghttp2_settings_entry iv[3];
+  nghttp2_mem *mem;
+  nghttp2_outbound_item *item;
+
+  mem = nghttp2_mem_default();
+  frame_pack_bufs_init(&bufs);
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.on_frame_recv_callback = on_frame_recv_callback;
+  callbacks.send_callback = null_send_callback;
+
+  nghttp2_option_new(&option);
+  nghttp2_option_set_max_settings(option, 1);
+
+  nghttp2_session_client_new2(&session, &callbacks, &ud, option);
+
+  CU_ASSERT(1 == session->max_settings);
+
+  nghttp2_option_del(option);
+
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 3000;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
+  iv[1].value = 16384;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, dup_iv(iv, 2),
+                              2);
+
+  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_settings_free(&frame.settings, mem);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
+  CU_ASSERT((ssize_t)nghttp2_buf_len(buf) == rv);
+
+  item = nghttp2_session_get_next_ob_item(session);
+  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
+
+  nghttp2_bufs_reset(&bufs);
+  nghttp2_bufs_free(&bufs);
+  nghttp2_session_del(session);
+}
+
 static void
 prepare_session_removed_closed_stream(nghttp2_session *session,
                                       nghttp2_hd_deflater *deflater) {
   int rv;
   nghttp2_settings_entry iv;
diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h
index e872c5d0..818c808d 100644
--- a/tests/nghttp2_session_test.h
+++ b/tests/nghttp2_session_test.h
@@ -154,10 +154,11 @@ void test_nghttp2_session_change_stream_priority(void);
 void test_nghttp2_session_create_idle_stream(void);
 void test_nghttp2_session_repeated_priority_change(void);
 void test_nghttp2_session_repeated_priority_submission(void);
 void test_nghttp2_session_set_local_window_size(void);
 void test_nghttp2_session_cancel_from_before_frame_send(void);
+void test_nghttp2_session_too_many_settings(void);
 void test_nghttp2_session_removed_closed_stream(void);
 void test_nghttp2_session_pause_data(void);
 void test_nghttp2_session_no_closed_streams(void);
 void test_nghttp2_session_set_stream_user_data(void);
 void test_nghttp2_http_mandatory_headers(void);

Fixed code:   nghttp2_option_set_max_settings.rst
	nghttp2_option_set_max_settings.rst \
/**
 * @macro
 *
 * The default max number of settings per SETTINGS frame
 */
#define NGHTTP2_DEFAULT_MAX_SETTINGS 32

  /**
   * When a local endpoint receives too many settings entries
   * in a single SETTINGS frame.
   */
  NGHTTP2_ERR_TOO_MANY_SETTINGS = -537,
/**
 * @function
 *
 * This function sets the maximum number of SETTINGS entries per
 * SETTINGS frame that will be accepted. If more than those entries
 * are received, the peer is considered to be misbehaving and session
 * will be closed. The default value is 32.
 */
NGHTTP2_EXTERN void nghttp2_option_set_max_settings(nghttp2_option *option,
                                                    size_t val);

  case NGHTTP2_ERR_TOO_MANY_SETTINGS:
    return "SETTINGS frame contained more than the maximum allowed entries";

void nghttp2_option_set_max_settings(nghttp2_option *option, size_t val) {
  option->opt_set_mask |= NGHTTP2_OPT_MAX_SETTINGS;
  option->max_settings = val;
}
  NGHTTP2_OPT_MAX_SETTINGS = 1 << 12,
  /**
   * NGHTTP2_OPT_MAX_SETTINGS
   */
  size_t max_settings;
  (*session_ptr)->max_settings = NGHTTP2_DEFAULT_MAX_SETTINGS;

    if ((option->opt_set_mask & NGHTTP2_OPT_MAX_SETTINGS) &&
        option->max_settings) {
      (*session_ptr)->max_settings = option->max_settings;
    }
          if (iframe->max_niv - 1 > session->max_settings) {
            rv = nghttp2_session_terminate_session_with_reason(
                session, NGHTTP2_ENHANCE_YOUR_CALM,
                "SETTINGS: too many setting entries");
            if (nghttp2_is_fatal(rv)) {
              return rv;
            }
            return (ssize_t)inlen;
          }

  /* SETTINGS frame contains too many settings */
  if (settings_payloadlen / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH
        > session->max_settings) {
    return NGHTTP2_ERR_TOO_MANY_SETTINGS;
  }
  /* The maximum number of settings accepted per SETTINGS frame. */
  size_t max_settings;
      !CU_add_test(pSuite, "session_too_many_settings",
                   test_nghttp2_session_too_many_settings) ||
void test_nghttp2_session_too_many_settings(void) {
  nghttp2_session *session;
  nghttp2_option *option;
  nghttp2_session_callbacks callbacks;
  nghttp2_frame frame;
  nghttp2_bufs bufs;
  nghttp2_buf *buf;
  ssize_t rv;
  my_user_data ud;
  nghttp2_settings_entry iv[3];
  nghttp2_mem *mem;
  nghttp2_outbound_item *item;

  mem = nghttp2_mem_default();
  frame_pack_bufs_init(&bufs);

  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
  callbacks.on_frame_recv_callback = on_frame_recv_callback;
  callbacks.send_callback = null_send_callback;

  nghttp2_option_new(&option);
  nghttp2_option_set_max_settings(option, 1);

  nghttp2_session_client_new2(&session, &callbacks, &ud, option);

  CU_ASSERT(1 == session->max_settings);

  nghttp2_option_del(option);

  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
  iv[0].value = 3000;

  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
  iv[1].value = 16384;

  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, dup_iv(iv, 2),
                              2);

  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);

  CU_ASSERT(0 == rv);
  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);

  nghttp2_frame_settings_free(&frame.settings, mem);

  buf = &bufs.head->buf;
  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));

  ud.frame_recv_cb_called = 0;

  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
  CU_ASSERT((ssize_t)nghttp2_buf_len(buf) == rv);

  item = nghttp2_session_get_next_ob_item(session);
  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);

  nghttp2_bufs_reset(&bufs);
  nghttp2_bufs_free(&bufs);
  nghttp2_session_del(session);
}

void test_nghttp2_session_too_many_settings(void);