From 3624ce6cfd4d197db75f01ae4be37723d7d9b638 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sat, 4 Jun 2016 09:53:23 -0400 Subject: [PATCH] fixed crash on invalid input in http_parser (#782) fixed crash on invalid input to http_parser --- ChangeLog | 1 + include/libtorrent/add_torrent_params.hpp | 3 +- src/http_parser.cpp | 32 +++++ test/test_http_parser.cpp | 139 ++++++++++++++++++---- 4 files changed, 151 insertions(+), 24 deletions(-) diff --git a/ChangeLog b/ChangeLog index 224ca5d92..7872ad1ed 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.1 release + * fixed crash on invalid input in http_parser * upgraded to libtommath 1.0 * fixed parsing of IPv6 endpoint with invalid port character separator * added limited support for new x.pe parameter from BEP 9 diff --git a/include/libtorrent/add_torrent_params.hpp b/include/libtorrent/add_torrent_params.hpp index 085b6f844..61ebc4a11 100644 --- a/include/libtorrent/add_torrent_params.hpp +++ b/include/libtorrent/add_torrent_params.hpp @@ -306,7 +306,7 @@ namespace libtorrent // also be stored in resume data. If you want the save path saved in // the resume data to be used, you need to set the // flag_use_resume_save_path flag. - // + // // .. note:: // On windows this path (and other paths) are interpreted as UNC // paths. This means they must use backslashes as directory separators @@ -377,6 +377,7 @@ namespace libtorrent // flags controlling aspects of this torrent and how it's added. See // flags_t for details. + // // .. note:: // The ``flags`` field is initialized with default flags by the // constructor. In order to preserve default behavior when clearing or diff --git a/src/http_parser.cpp b/src/http_parser.cpp index a9497f80f..52f6152fd 100644 --- a/src/http_parser.cpp +++ b/src/http_parser.cpp @@ -174,6 +174,7 @@ restart_response: if (m_state == read_status) { TORRENT_ASSERT(!m_finished); + TORRENT_ASSERT(pos <= recv_buffer.end); char const* newline = std::find(pos, recv_buffer.end, '\n'); // if we don't have a full line yet, wait. if (newline == recv_buffer.end) @@ -194,6 +195,7 @@ restart_response: char const* line = pos; ++newline; + TORRENT_ASSERT(newline >= pos); int incoming = int(newline - pos); m_recv_pos += incoming; boost::get<1>(ret) += newline - (m_recv_buffer.begin + start_pos); @@ -227,6 +229,7 @@ restart_response: if (m_state == read_header) { TORRENT_ASSERT(!m_finished); + TORRENT_ASSERT(pos <= recv_buffer.end); char const* newline = std::find(pos, recv_buffer.end, '\n'); std::string line; @@ -277,6 +280,12 @@ restart_response: if (name == "content-length") { m_content_length = strtoll(value.c_str(), 0, 10); + if (m_content_length < 0) + { + m_state = error_state; + error = true; + return ret; + } } else if (name == "connection") { @@ -294,12 +303,24 @@ restart_response: if (string_begins_no_case("bytes ", ptr)) ptr += 6; char* end; m_range_start = strtoll(ptr, &end, 10); + if (m_range_start < 0) + { + m_state = error_state; + error = true; + return ret; + } if (end == ptr) success = false; else if (*end != '-') success = false; else { ptr = end + 1; m_range_end = strtoll(ptr, &end, 10); + if (m_range_end < 0) + { + m_state = error_state; + error = true; + return ret; + } if (end == ptr) success = false; } @@ -318,6 +339,7 @@ restart_response: } TORRENT_ASSERT(m_recv_pos <= recv_buffer.left()); + TORRENT_ASSERT(pos <= recv_buffer.end); newline = std::find(pos, recv_buffer.end, '\n'); } boost::get<1>(ret) += newline - (m_recv_buffer.begin + start_pos); @@ -347,6 +369,12 @@ restart_response: int header_size; if (parse_chunk_header(buf, &chunk_size, &header_size)) { + if (chunk_size < 0) + { + m_state = error_state; + error = true; + return ret; + } if (chunk_size > 0) { std::pair chunk_range(m_cur_chunk_end + header_size @@ -419,6 +447,7 @@ restart_response: bool http_parser::parse_chunk_header(buffer::const_interval buf , boost::int64_t* chunk_size, int* header_size) { + TORRENT_ASSERT(buf.begin <= buf.end); char const* pos = buf.begin; // ignore one optional new-line. This is since each chunk @@ -429,6 +458,7 @@ restart_response: if (pos < buf.end && pos[0] == '\n') ++pos; if (pos == buf.end) return false; + TORRENT_ASSERT(pos <= buf.end); char const* newline = std::find(pos, buf.end, '\n'); if (newline == buf.end) return false; ++newline; @@ -441,6 +471,8 @@ restart_response: // first, read the chunk length *chunk_size = strtoll(pos, 0, 16); + if (*chunk_size < 0) return true; + if (*chunk_size != 0) { *header_size = newline - buf.begin; diff --git a/test/test_http_parser.cpp b/test/test_http_parser.cpp index c26d1c8b1..6835a121d 100644 --- a/test/test_http_parser.cpp +++ b/test/test_http_parser.cpp @@ -361,29 +361,6 @@ TORRENT_TEST(http_parser) TEST_EQUAL(parser.headers().find("test2")->second, "bar"); } - // test chunked encoding - - parser.reset(); - - char const* chunked_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"; - received = feed_bytes(parser, chunked_input); - - TEST_EQUAL(strlen(chunked_input), 24 + 94) - TEST_CHECK(received == make_tuple(24, 94, false)); - TEST_CHECK(parser.finished()); - - char mutable_buffer[100]; - memcpy(mutable_buffer, parser.get_body().begin, parser.get_body().left()); - int len = parser.collapse_chunk_headers(mutable_buffer, parser.get_body().left()); - - TEST_CHECK(std::equal(mutable_buffer, mutable_buffer + len, "test12340123456789abcdef")); - // test url parsing error_code ec; @@ -476,3 +453,119 @@ TORRENT_TEST(http_parser) TEST_EQUAL(is_redirect(400), false); } +TORRENT_TEST(chunked_encoding) +{ + char const* chunked_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"; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, chunked_input); + + TEST_EQUAL(strlen(chunked_input), 24 + 94) + TEST_CHECK(received == make_tuple(24, 94, false)); + TEST_CHECK(parser.finished()); + + char mutable_buffer[100]; + memcpy(mutable_buffer, parser.get_body().begin, parser.get_body().left()); + int len = parser.collapse_chunk_headers(mutable_buffer, parser.get_body().left()); + + TEST_CHECK(std::equal(mutable_buffer, mutable_buffer + len, "test12340123456789abcdef")); +} + +TORRENT_TEST(invalid_content_length) +{ + char const* chunked_input = + "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "Content-Length: -45345\r\n" + "\r\n"; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, chunked_input); + + TEST_CHECK(boost::get<2>(received) == true); +} + +TORRENT_TEST(invalid_chunked) +{ + char const* chunked_input = + "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "-53465234545\r\n" + "foobar"; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, chunked_input); + + TEST_CHECK(boost::get<2>(received) == true); +} + +TORRENT_TEST(invalid_content_range_start) +{ + char const* chunked_input = + "HTTP/1.1 206 OK\n" + "Content-Range: bYTes -3-4\n" + "\n"; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, chunked_input); + + TEST_CHECK(boost::get<2>(received) == true); +} + +TORRENT_TEST(invalid_content_range_end) +{ + char const* chunked_input = + "HTTP/1.1 206 OK\n" + "Content-Range: bYTes 3--434\n" + "\n"; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, chunked_input); + + TEST_CHECK(boost::get<2>(received) == true); +} + +TORRENT_TEST(invalid_chunk_afl) +{ + boost::uint8_t const invalid_chunked_input[] = { + 0x48, 0x6f, 0x54, 0x50, 0x2f, 0x31, 0x2e, 0x31, // HoTP/1.1 200 OK + 0x20, 0x32, 0x30, 0x30, 0x20, 0x4f, 0x4b, 0x0d, // Cont-Length: 20 + 0x0a, 0x43, 0x6f, 0x6e, 0x74, 0x2d, 0x4c, 0x65, // Contente: tn + 0x6e, 0x67, 0x74, 0x68, 0x3a, 0x20, 0x32, 0x30, // Transfer-Encoding: chunked + 0x0d, 0x0a, 0x43, 0x6f, 0x6e, 0x74, 0x65, 0x6e, // + 0x74, 0x65, 0x3a, 0x20, 0x74, 0x6e, 0x0d, 0x0a, // + 0x54, 0x72, 0x61, 0x6e, 0x73, 0x66, 0x65, 0x72, // + 0x2d, 0x45, 0x6e, 0x63, 0x6f, 0x64, 0x69, 0x6e, // -89abc9abcdef + 0x67, 0x3a, 0x20, 0x63, 0x68, 0x75, 0x6e, 0x6b, // � + 0x65, 0x64, 0x0d, 0x0a, 0x0d, 0x0d, 0x0a, 0x0d, // T����������def + 0x0a, 0x0a, 0x2d, 0x38, 0x39, 0x61, 0x62, 0x63, // � + 0x39, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x0d, // T�����������est-headyr: foobar + 0x0a, 0xd6, 0x0d, 0x0a, 0x54, 0xbd, 0xbd, 0xbd, + 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0x64, + 0x65, 0x66, 0x0d, 0x0a, 0xd6, 0x0d, 0x0a, 0x54, + 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, 0xbd, + 0xbd, 0xbd, 0xbd, 0x65, 0x73, 0x74, 0x2d, 0x68, + 0x65, 0x61, 0x64, 0x79, 0x72, 0x3a, 0x20, 0x66, + 0x6f, 0x6f, 0x62, 0x61, 0x72, 0x0d, 0x0a, 0x0d, + 0x0a, 0x00 + }; + + http_parser parser; + boost::tuple const received + = feed_bytes(parser, reinterpret_cast(invalid_chunked_input)); + + TEST_CHECK(boost::get<2>(received) == true); +} +