From 06379b286184e001d8610a5a72326f7d2b9cdf78 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 22 Apr 2018 12:00:55 +0900 Subject: [PATCH] Fix treatment of padding --- lib/nghttp2_session.c | 16 +++++-- tests/main.c | 2 + tests/nghttp2_session_test.c | 88 ++++++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 4 files changed, 103 insertions(+), 4 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index c58f059b..4953ab70 100644 --- 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 index a922da6e..9fe0bfdb 100644 --- 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 index ada4c8b4..e54e5afd 100644 --- 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 index 470b274c..9af6ea99 100644 --- 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);