From ed46298820bb128bd05bb1d2c74d074e17cc62e6 Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 21 May 2021 17:44:05 +0200 Subject: [PATCH] fix bug in chunked encoding --- ChangeLog | 1 + src/http_parser.cpp | 10 +++-- test/test_http_parser.cpp | 89 ++++++++++++++++++++++++++++++++------- 3 files changed, 81 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index ac6b67face1..9a8420e1247 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * fix bug in parsing chunked encoding * fix incorrect reporting of active_duration when entering graceful-pause * fix python binding for functions taking string_view * fix python binding for torrent_info constructor overloads diff --git a/src/http_parser.cpp b/src/http_parser.cpp index 8f348b0065f..c6fb7097c7c 100644 --- a/src/http_parser.cpp +++ b/src/http_parser.cpp @@ -566,7 +566,7 @@ namespace libtorrent { span http_parser::get_body() const { - TORRENT_ASSERT(m_state == read_body); + if (m_state != read_body) return {}; std::int64_t const received = m_recv_pos - m_body_start_pos; std::int64_t const body_length = m_chunked_encoding && !m_chunked_ranges.empty() @@ -612,8 +612,12 @@ namespace libtorrent { { auto const chunk_start = i.first; auto const chunk_end = i.second; - TORRENT_ASSERT(i.second - i.first < std::numeric_limits::max()); - TORRENT_ASSERT(chunk_end - offset <= buffer.size()); + if (chunk_end - offset > buffer.size() + || (i.second - i.first) >= std::numeric_limits::max()) + { + // invalid chunk header. Return the body we've parsed out so far + return buffer.first(write_ptr - buffer.data()); + } span chunk = buffer.subspan( aux::numeric_cast(chunk_start - offset) , aux::numeric_cast(chunk_end - chunk_start)); diff --git a/test/test_http_parser.cpp b/test/test_http_parser.cpp index c6003a88c5b..7eea6c9c58c 100644 --- a/test/test_http_parser.cpp +++ b/test/test_http_parser.cpp @@ -536,30 +536,87 @@ TORRENT_TEST(http_parser) TEST_EQUAL(is_redirect(400), false); } -TORRENT_TEST(chunked_encoding) +std::string test_collapse_chunks(std::string chunked_input, bool const expect_error = false) { - char const chunked_input[] = - "HTTP/1.1 200 OK\r\n" + http_parser parser; + + std::string input = "HTTP/1.1 200 OK\r\n" "Transfer-Encoding: chunked\r\n" "Content-Type: text/plain\r\n" - "\r\n" - "4\r\ntest\r\n4\r\n1234\r\n10\r\n0123456789abcdef\r\n" - "0\r\n\r\n"; + "\r\n"; - http_parser parser; - std::tuple const received - = feed_bytes(parser, chunked_input); + input += chunked_input; - TEST_EQUAL(strlen(chunked_input), 24 + 94); - TEST_CHECK(received == std::make_tuple(24, 94, false)); - TEST_CHECK(parser.finished()); + bool error = false; + int payload = 0; + int protocol = 0; + std::tie(payload, protocol) = parser.incoming(input, error); - char mutable_buffer[100]; + TEST_CHECK(protocol > 0); + TEST_CHECK(payload > 0); + if (expect_error) + { + TEST_CHECK(std::size_t(protocol + payload) <= input.size()); + } + else + { + TEST_EQUAL(std::size_t(protocol + payload), input.size()); + TEST_CHECK(parser.finished()); + } + + std::vector mutable_buffer; span body = parser.get_body(); - std::copy(body.begin(), body.end(), mutable_buffer); - body = parser.collapse_chunk_headers({mutable_buffer, body.size()}); + std::copy(body.begin(), body.end(), std::back_inserter(mutable_buffer)); + body = parser.collapse_chunk_headers(mutable_buffer); + return std::string(body.data(), body.size()); +} - TEST_CHECK(body == span("test12340123456789abcdef", 24)); +TORRENT_TEST(chunked_encoding) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n10\r\n0123456789abcdef\r\n" + "0\r\n\r\n"); + TEST_EQUAL(collapsed, "test12340123456789abcdef"); +} + +TORRENT_TEST(chunked_encoding_beyond_end) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n20\r\n0123456789abcdef\r\n" + "0\r\n\r\n", true); + TEST_EQUAL(collapsed, "test1234"); +} + +TORRENT_TEST(chunked_encoding_end_of_buffer) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n17\r\n0123456789abcdef\r\n" + "0\r\n\r\n", true); + TEST_EQUAL(collapsed, "test12340123456789abcdef\r\n0\r\n\r\n"); +} + +TORRENT_TEST(chunked_encoding_past_end) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n18\r\n0123456789abcdef\r\n" + "0\r\n\r\n", true); + TEST_EQUAL(collapsed, "test1234"); +} + +TORRENT_TEST(chunked_encoding_negative) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n-10\r\n0123456789abcdef\r\n" + "0\r\n\r\n", true); + TEST_EQUAL(collapsed, ""); +} + +TORRENT_TEST(chunked_encoding_end) +{ + auto const collapsed = test_collapse_chunks( + "4\r\ntest\r\n4\r\n1234\r\n11\r\n0123456789abcdef\r\n" + "0\r\n\r\n", true); + TEST_EQUAL(collapsed, "test12340123456789abcdef\r"); } TORRENT_TEST(chunked_encoding_overflow)