From cfb9857f27e03ac4ca9e8267db335f19b59e3b4a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 27 Oct 2013 20:55:44 +0900 Subject: [PATCH] Use FRAME_SIZE_ERROR for invalid payload length --- lib/nghttp2_frame.c | 22 +++++++++++--------- lib/nghttp2_frame.h | 40 ++++++++++++++++++------------------ lib/nghttp2_session.c | 26 ++++++++++++----------- tests/nghttp2_session_test.c | 2 +- 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 693f9c8c..e42a3708 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -273,7 +273,7 @@ int nghttp2_frame_unpack_headers_without_nv(nghttp2_headers *frame, } if(head[3] & NGHTTP2_FLAG_PRIORITY) { if(payloadlen < 4) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } frame->pri = nghttp2_get_uint32(payload) & NGHTTP2_PRIORITY_MASK; } else { @@ -304,7 +304,7 @@ int nghttp2_frame_unpack_priority(nghttp2_priority *frame, const uint8_t *payload, size_t payloadlen) { if(payloadlen != 4) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } nghttp2_frame_unpack_frame_hd(&frame->hd, head); frame->pri = nghttp2_get_uint32(payload) & NGHTTP2_PRIORITY_MASK; @@ -332,7 +332,7 @@ int nghttp2_frame_unpack_rst_stream(nghttp2_rst_stream *frame, const uint8_t *payload, size_t payloadlen) { if(payloadlen != 4) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } nghttp2_frame_unpack_frame_hd(&frame->hd, head); frame->error_code = nghttp2_get_uint32(payload); @@ -371,10 +371,12 @@ int nghttp2_frame_unpack_settings(nghttp2_settings *frame, const uint8_t *payload, size_t payloadlen) { int rv; - if(payloadlen % 8) { - return NGHTTP2_ERR_INVALID_FRAME; - } nghttp2_frame_unpack_frame_hd(&frame->hd, head); + if((payloadlen & 0x7) || + ((frame->hd.flags & NGHTTP2_FLAG_ACK) && payloadlen > 0)) { + return NGHTTP2_ERR_FRAME_SIZE_ERROR; + } + rv = nghttp2_frame_unpack_settings_payload(&frame->iv, &frame->niv, payload, payloadlen); if(rv != 0) { @@ -464,7 +466,7 @@ int nghttp2_frame_unpack_push_promise_without_nv(nghttp2_push_promise *frame, return NGHTTP2_ERR_PROTO; } if(payloadlen < 4) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } frame->promised_stream_id = nghttp2_get_uint32(payload) & NGHTTP2_STREAM_ID_MASK; @@ -493,7 +495,7 @@ int nghttp2_frame_unpack_ping(nghttp2_ping *frame, const uint8_t *payload, size_t payloadlen) { if(payloadlen != 8) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } nghttp2_frame_unpack_frame_hd(&frame->hd, head); memcpy(frame->opaque_data, payload, sizeof(frame->opaque_data)); @@ -524,7 +526,7 @@ int nghttp2_frame_unpack_goaway(nghttp2_goaway *frame, { nghttp2_frame_unpack_frame_hd(&frame->hd, head); if(payloadlen < 8) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } frame->last_stream_id = nghttp2_get_uint32(payload) & NGHTTP2_STREAM_ID_MASK; frame->error_code = nghttp2_get_uint32(payload+4); @@ -562,7 +564,7 @@ int nghttp2_frame_unpack_window_update(nghttp2_window_update *frame, size_t payloadlen) { if(payloadlen != 4) { - return NGHTTP2_ERR_INVALID_FRAME; + return NGHTTP2_ERR_FRAME_SIZE_ERROR; } nghttp2_frame_unpack_frame_hd(&frame->hd, head); frame->window_size_increment = nghttp2_get_uint32(payload) & diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 54814a8b..089098c9 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -132,8 +132,8 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr, * The inflate operation failed. * NGHTTP2_ERR_INVALID_HEADER_BLOCK * Unpacking succeeds but the header block is invalid. - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid * NGHTTP2_ERR_NOMEM * Out of memory. */ @@ -149,8 +149,8 @@ int nghttp2_frame_unpack_headers(nghttp2_headers *frame, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_headers_without_nv(nghttp2_headers *frame, const uint8_t *head, @@ -179,8 +179,8 @@ ssize_t nghttp2_frame_pack_priority(uint8_t **buf_ptr, size_t *buflen_ptr, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_priority(nghttp2_priority *frame, const uint8_t *head, size_t headlen, @@ -208,8 +208,8 @@ ssize_t nghttp2_frame_pack_rst_stream(uint8_t **buf_ptr, size_t *buflen_ptr, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_rst_stream(nghttp2_rst_stream *frame, const uint8_t *head, size_t headlen, @@ -246,8 +246,8 @@ size_t nghttp2_frame_pack_settings_payload(uint8_t *buf, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid * NGHTTP2_ERR_NOMEM * Out of memory. */ @@ -317,8 +317,8 @@ ssize_t nghttp2_frame_pack_push_promise(uint8_t **buf_ptr, * The inflate operation failed. * NGHTTP2_ERR_INVALID_HEADER_BLOCK * Unpacking succeeds but the header block is invalid. - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid * NGHTTP2_ERR_NOMEM * Out of memory. */ @@ -335,8 +335,8 @@ int nghttp2_frame_unpack_push_promise(nghttp2_push_promise *frame, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_push_promise_without_nv(nghttp2_push_promise *frame, const uint8_t *head, @@ -365,8 +365,8 @@ ssize_t nghttp2_frame_pack_ping(uint8_t **buf_ptr, size_t *buflen_ptr, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_ping(nghttp2_ping *frame, const uint8_t *head, size_t headlen, @@ -393,8 +393,8 @@ ssize_t nghttp2_frame_pack_goaway(uint8_t **buf_ptr, size_t *buflen_ptr, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid * NGHTTP2_ERR_NOMEM * Out of memory. */ @@ -423,8 +423,8 @@ ssize_t nghttp2_frame_pack_window_update(uint8_t **buf_ptr, size_t *buflen_ptr, * This function returns 0 if it succeeds or one of the following * negative error codes: * - * NGHTTP2_ERR_INVALID_FRAME - * The input data are invalid. + * NGHTTP2_ERR_FRAME_SIZE_ERROR + * The input length is invalid */ int nghttp2_frame_unpack_window_update(nghttp2_window_update *frame, const uint8_t *head, size_t headlen, diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index bd6d4d34..41307b38 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2684,6 +2684,8 @@ static int get_error_code_from_lib_error_code(int lib_error_code) switch(lib_error_code) { case NGHTTP2_ERR_HEADER_COMP: return NGHTTP2_COMPRESSION_ERROR; + case NGHTTP2_ERR_FRAME_SIZE_ERROR: + return NGHTTP2_FRAME_SIZE_ERROR; default: return NGHTTP2_PROTOCOL_ERROR; } @@ -2764,8 +2766,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_priority_free(&frame->priority); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; case NGHTTP2_RST_STREAM: @@ -2780,8 +2782,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_rst_stream_free(&frame->rst_stream); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; case NGHTTP2_SETTINGS: @@ -2796,8 +2798,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_settings_free(&frame->settings); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; case NGHTTP2_PUSH_PROMISE: @@ -2834,8 +2836,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_ping_free(&frame->ping); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; case NGHTTP2_GOAWAY: @@ -2850,8 +2852,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_goaway_free(&frame->goaway); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; case NGHTTP2_WINDOW_UPDATE: @@ -2866,8 +2868,8 @@ static int nghttp2_session_process_ctrl_frame(nghttp2_session *session) nghttp2_frame_window_update_free(&frame->window_update); } } else if(nghttp2_is_non_fatal(r)) { - r = nghttp2_session_handle_parse_error(session, type, r, - NGHTTP2_PROTOCOL_ERROR); + r = nghttp2_session_handle_parse_error + (session, type, r, get_error_code_from_lib_error_code(r)); } break; default: diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 4af2931d..7ac8a7bb 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -463,7 +463,7 @@ void test_nghttp2_session_recv(void) CU_ASSERT(0 == user_data.frame_recv_cb_called); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_GOAWAY == OB_CTRL_TYPE(item)); - CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == OB_CTRL(item)->goaway.error_code); + CU_ASSERT(NGHTTP2_FRAME_SIZE_ERROR == OB_CTRL(item)->goaway.error_code); CU_ASSERT(0 == nghttp2_session_send(session)); free(framedata);