Fix treatment of padding

boostiewj · Apr 22, 2018 · 06379b2 · 06379b2
1 parent e04de48
commit 06379b2
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 4 deletions.
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
@@ -5800,8 +5800,10 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
       case NGHTTP2_HEADERS:
         if (iframe->padlen == 0 &&
             (iframe->frame.hd.flags & NGHTTP2_FLAG_PADDED)) {
+          pri_fieldlen = nghttp2_frame_priority_len(iframe->frame.hd.flags);
           padlen = inbound_frame_compute_pad(iframe);
-          if (padlen < 0) {
+          if (padlen < 0 ||
+              (size_t)padlen + pri_fieldlen > 1 + iframe->payloadleft) {
             busy = 1;
             rv = nghttp2_session_terminate_session_with_reason(
                 session, NGHTTP2_PROTOCOL_ERROR, "HEADERS: invalid padding");
@@ -5813,8 +5815,6 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
           }
           iframe->frame.headers.padlen = (size_t)padlen;
 
-          pri_fieldlen = nghttp2_frame_priority_len(iframe->frame.hd.flags);
-
           if (pri_fieldlen > 0) {
             if (iframe->payloadleft < pri_fieldlen) {
               busy = 1;
@@ -5877,8 +5877,10 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
         if (iframe->padlen == 0 &&
             (iframe->frame.hd.flags & NGHTTP2_FLAG_PADDED)) {
           padlen = inbound_frame_compute_pad(iframe);
-          if (padlen < 0) {
+          if (padlen < 0 || (size_t)padlen + 4 /* promised stream id */
+                                > 1 + iframe->payloadleft) {
             busy = 1;
+
             rv = nghttp2_session_terminate_session_with_reason(
                 session, NGHTTP2_PROTOCOL_ERROR,
                 "PUSH_PROMISE: invalid padding");
@@ -6028,6 +6030,12 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
 
       data_readlen = inbound_frame_effective_readlen(
           iframe, iframe->payloadleft - readlen, readlen);
+
+      if (data_readlen == -1) {
+        /* everything is padding */
+        data_readlen = 0;
+      }
+
       trail_padlen = nghttp2_frame_trail_padlen(&iframe->frame, iframe->padlen);
 
       final = (iframe->frame.hd.flags & NGHTTP2_FLAG_END_HEADERS) &&

diff --git a/tests/main.c b/tests/main.c
@@ -87,6 +87,8 @@ int main() {
                    test_nghttp2_session_recv_continuation) ||
       !CU_add_test(pSuite, "session_recv_headers_with_priority",
                    test_nghttp2_session_recv_headers_with_priority) ||
+      !CU_add_test(pSuite, "session_recv_headers_with_padding",
+                   test_nghttp2_session_recv_headers_with_padding) ||
       !CU_add_test(pSuite, "session_recv_headers_early_response",
                    test_nghttp2_session_recv_headers_early_response) ||
       !CU_add_test(pSuite, "session_server_recv_push_response",

diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
@@ -1575,6 +1575,94 @@ void test_nghttp2_session_recv_headers_with_priority(void) {
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_session_recv_headers_with_padding(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_bufs bufs;
+  nghttp2_buf *buf;
+  nghttp2_frame_hd hd;
+  nghttp2_outbound_item *item;
+  my_user_data ud;
+  ssize_t rv;
+
+  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;
+
+  /* HEADERS: Wrong padding length */
+  nghttp2_session_server_new(&session, &callbacks, &ud);
+  nghttp2_session_send(session);
+
+  nghttp2_frame_hd_init(&hd, 10, NGHTTP2_HEADERS,
+                        NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY |
+                            NGHTTP2_FLAG_PADDED,
+                        1);
+  buf = &bufs.head->buf;
+  nghttp2_frame_pack_frame_hd(buf->last, &hd);
+  buf->last += NGHTTP2_FRAME_HDLEN;
+  /* padding is 6 bytes */
+  *buf->last++ = 5;
+  /* priority field */
+  nghttp2_put_uint32be(buf->last, 3);
+  buf->last += sizeof(uint32_t);
+  *buf->last++ = 1;
+  /* rest is garbage */
+  memset(buf->last, 0, 4);
+  buf->last += 4;
+
+  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);
+  CU_ASSERT(0 == ud.frame_recv_cb_called);
+
+  item = nghttp2_session_get_next_ob_item(session);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
+
+  nghttp2_bufs_reset(&bufs);
+  nghttp2_session_del(session);
+
+  /* PUSH_PROMISE: Wrong padding length */
+  nghttp2_session_client_new(&session, &callbacks, &ud);
+  nghttp2_session_send(session);
+
+  open_sent_stream(session, 1);
+
+  nghttp2_frame_hd_init(&hd, 9, NGHTTP2_PUSH_PROMISE,
+                        NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PADDED, 1);
+  buf = &bufs.head->buf;
+  nghttp2_frame_pack_frame_hd(buf->last, &hd);
+  buf->last += NGHTTP2_FRAME_HDLEN;
+  /* padding is 6 bytes */
+  *buf->last++ = 5;
+  /* promised stream ID field */
+  nghttp2_put_uint32be(buf->last, 2);
+  buf->last += sizeof(uint32_t);
+  /* rest is garbage */
+  memset(buf->last, 0, 4);
+  buf->last += 4;
+
+  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);
+  CU_ASSERT(0 == ud.frame_recv_cb_called);
+
+  item = nghttp2_session_get_next_ob_item(session);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
+
+  nghttp2_bufs_free(&bufs);
+  nghttp2_session_del(session);
+}
+
 static int response_on_begin_frame_callback(nghttp2_session *session,
                                             const nghttp2_frame_hd *hd,
                                             void *user_data) {

diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h
@@ -37,6 +37,7 @@ void test_nghttp2_session_recv_data(void);
 void test_nghttp2_session_recv_data_no_auto_flow_control(void);
 void test_nghttp2_session_recv_continuation(void);
 void test_nghttp2_session_recv_headers_with_priority(void);
+void test_nghttp2_session_recv_headers_with_padding(void);
 void test_nghttp2_session_recv_headers_early_response(void);
 void test_nghttp2_session_server_recv_push_response(void);
 void test_nghttp2_session_recv_premature_headers(void);