diff --git a/ChangeLog b/ChangeLog index b9ea29ffb..6573216cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -45,6 +45,8 @@ * fix uTP edge case where udp socket buffer fills up * fix nagle implementation in uTP + * improve error checking in lazy_bdecode + 0.16.16 release * add missing add_files overload to the python bindings diff --git a/include/libtorrent/error_code.hpp b/include/libtorrent/error_code.hpp index 07aff2056..50f70a2fd 100644 --- a/include/libtorrent/error_code.hpp +++ b/include/libtorrent/error_code.hpp @@ -431,6 +431,8 @@ namespace libtorrent depth_exceeded, // bencoded item count limit exceeded limit_exceeded, + // integer overflow + overflow, #endif // the number of error codes diff --git a/include/libtorrent/lazy_entry.hpp b/include/libtorrent/lazy_entry.hpp index 8cc9e792f..a49daec15 100644 --- a/include/libtorrent/lazy_entry.hpp +++ b/include/libtorrent/lazy_entry.hpp @@ -412,6 +412,8 @@ namespace libtorrent depth_exceeded, // bencoded item count limit exceeded limit_exceeded, + // integer overflow + overflow, // the number of error codes error_code_max diff --git a/src/error_code.cpp b/src/error_code.cpp index f9c5c0a11..b9091ef3f 100644 --- a/src/error_code.cpp +++ b/src/error_code.cpp @@ -266,6 +266,7 @@ namespace libtorrent "expected value (list, dict, int or string) in bencoded string", "bencoded nesting depth exceeded", "bencoded item count limit exceeded", + "integer overflow", #endif }; if (ev < 0 || ev >= int(sizeof(msgs)/sizeof(msgs[0]))) diff --git a/src/lazy_bdecode.cpp b/src/lazy_bdecode.cpp index cafda7efc..665d5a673 100644 --- a/src/lazy_bdecode.cpp +++ b/src/lazy_bdecode.cpp @@ -71,8 +71,19 @@ namespace libtorrent ec = bdecode_errors::expected_string; return start; } + if (val > INT64_MAX / 10) + { + ec = bdecode_errors::overflow; + return start; + } val *= 10; - val += *start - '0'; + int digit = *start - '0'; + if (val > INT64_MAX - digit) + { + ec = bdecode_errors::overflow; + return start; + } + val += digit; ++start; } if (*start != delimiter) @@ -139,6 +150,9 @@ namespace libtorrent if (start + len + 1 > end) TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); + if (len < 0) + TORRENT_FAIL_BDECODE(bdecode_errors::overflow); + ++start; if (start == end) TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); lazy_entry* ent = top->dict_append(start); @@ -201,6 +215,9 @@ namespace libtorrent TORRENT_FAIL_BDECODE(e); if (start + len + 1 > end) TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); + if (len < 0) + TORRENT_FAIL_BDECODE(bdecode_errors::overflow); + ++start; top->construct_string(start, int(len)); stack.pop_back(); @@ -220,9 +237,9 @@ namespace libtorrent bool negative = false; if (*m_data.start == '-') negative = true; bdecode_errors::error_code_enum ec = bdecode_errors::no_error; - parse_int(negative?m_data.start+1:m_data.start + parse_int(m_data.start + negative , m_data.start + m_size, 'e', val, ec); - TORRENT_ASSERT(!ec); + if (ec) return 0; if (negative) val = -val; return val; } @@ -628,6 +645,7 @@ namespace libtorrent "expected value (list, dict, int or string) in bencoded string", "bencoded nesting depth exceeded", "bencoded item count limit exceeded", + "integer overflow", }; if (ev < 0 || ev >= int(sizeof(msgs)/sizeof(msgs[0]))) return "Unknown error"; diff --git a/test/test_bencoding.cpp b/test/test_bencoding.cpp index b990c0fac..a73c704bc 100644 --- a/test/test_bencoding.cpp +++ b/test/test_bencoding.cpp @@ -109,13 +109,13 @@ int test_main() lazy_entry e; error_code ec; int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); - TORRENT_ASSERT(ret == 0); + TEST_CHECK(ret == 0); printf("%s\n", print_entry(e).c_str()); std::pair section = e.data_section(); - TORRENT_ASSERT(std::memcmp(b, section.first, section.second) == 0); - TORRENT_ASSERT(section.second == sizeof(b) - 1); - TORRENT_ASSERT(e.type() == lazy_entry::int_t); - TORRENT_ASSERT(e.int_value() == 12453); + TEST_CHECK(std::memcmp(b, section.first, section.second) == 0); + TEST_CHECK(section.second == sizeof(b) - 1); + TEST_CHECK(e.type() == lazy_entry::int_t); + TEST_CHECK(e.int_value() == 12453); } { @@ -123,14 +123,14 @@ int test_main() lazy_entry e; error_code ec; int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); - TORRENT_ASSERT(ret == 0); + TEST_CHECK(ret == 0); printf("%s\n", print_entry(e).c_str()); std::pair section = e.data_section(); - TORRENT_ASSERT(std::memcmp(b, section.first, section.second) == 0); - TORRENT_ASSERT(section.second == sizeof(b) - 1); - TORRENT_ASSERT(e.type() == lazy_entry::string_t); - TORRENT_ASSERT(e.string_value() == std::string("abcdefghijklmnopqrstuvwxyz")); - TORRENT_ASSERT(e.string_length() == 26); + TEST_CHECK(std::memcmp(b, section.first, section.second) == 0); + TEST_CHECK(section.second == sizeof(b) - 1); + TEST_CHECK(e.type() == lazy_entry::string_t); + TEST_CHECK(e.string_value() == std::string("abcdefghijklmnopqrstuvwxyz")); + TEST_CHECK(e.string_length() == 26); } { @@ -138,21 +138,21 @@ int test_main() lazy_entry e; error_code ec; int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); - TORRENT_ASSERT(ret == 0); + TEST_CHECK(ret == 0); printf("%s\n", print_entry(e).c_str()); std::pair section = e.data_section(); - TORRENT_ASSERT(std::memcmp(b, section.first, section.second) == 0); - TORRENT_ASSERT(section.second == sizeof(b) - 1); - TORRENT_ASSERT(e.type() == lazy_entry::list_t); - TORRENT_ASSERT(e.list_size() == 2); - TORRENT_ASSERT(e.list_at(0)->type() == lazy_entry::int_t); - TORRENT_ASSERT(e.list_at(1)->type() == lazy_entry::string_t); - TORRENT_ASSERT(e.list_at(0)->int_value() == 12453); - TORRENT_ASSERT(e.list_at(1)->string_value() == std::string("aaa")); - TORRENT_ASSERT(e.list_at(1)->string_length() == 3); + TEST_CHECK(std::memcmp(b, section.first, section.second) == 0); + TEST_CHECK(section.second == sizeof(b) - 1); + TEST_CHECK(e.type() == lazy_entry::list_t); + TEST_CHECK(e.list_size() == 2); + TEST_CHECK(e.list_at(0)->type() == lazy_entry::int_t); + TEST_CHECK(e.list_at(1)->type() == lazy_entry::string_t); + TEST_CHECK(e.list_at(0)->int_value() == 12453); + TEST_CHECK(e.list_at(1)->string_value() == std::string("aaa")); + TEST_CHECK(e.list_at(1)->string_length() == 3); section = e.list_at(1)->data_section(); - TORRENT_ASSERT(std::memcmp("3:aaa", section.first, section.second) == 0); - TORRENT_ASSERT(section.second == 5); + TEST_CHECK(std::memcmp("3:aaa", section.first, section.second) == 0); + TEST_CHECK(section.second == 5); } { @@ -160,22 +160,82 @@ int test_main() lazy_entry e; error_code ec; int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); - TORRENT_ASSERT(ret == 0); + TEST_CHECK(ret == 0); printf("%s\n", print_entry(e).c_str()); std::pair section = e.data_section(); - TORRENT_ASSERT(std::memcmp(b, section.first, section.second) == 0); - TORRENT_ASSERT(section.second == sizeof(b) - 1); - TORRENT_ASSERT(e.type() == lazy_entry::dict_t); - TORRENT_ASSERT(e.dict_size() == 4); - TORRENT_ASSERT(e.dict_find("a")->type() == lazy_entry::int_t); - TORRENT_ASSERT(e.dict_find("a")->int_value() == 12453); - TORRENT_ASSERT(e.dict_find("b")->type() == lazy_entry::string_t); - TORRENT_ASSERT(e.dict_find("b")->string_value() == std::string("aaa")); - TORRENT_ASSERT(e.dict_find("b")->string_length() == 3); - TORRENT_ASSERT(e.dict_find("c")->type() == lazy_entry::string_t); - TORRENT_ASSERT(e.dict_find("c")->string_value() == std::string("bbb")); - TORRENT_ASSERT(e.dict_find("c")->string_length() == 3); - TORRENT_ASSERT(e.dict_find_string_value("X") == "0123456789"); + TEST_CHECK(std::memcmp(b, section.first, section.second) == 0); + TEST_CHECK(section.second == sizeof(b) - 1); + TEST_CHECK(e.type() == lazy_entry::dict_t); + TEST_CHECK(e.dict_size() == 4); + TEST_CHECK(e.dict_find("a")->type() == lazy_entry::int_t); + TEST_CHECK(e.dict_find("a")->int_value() == 12453); + TEST_CHECK(e.dict_find("b")->type() == lazy_entry::string_t); + TEST_CHECK(e.dict_find("b")->string_value() == std::string("aaa")); + TEST_CHECK(e.dict_find("b")->string_length() == 3); + TEST_CHECK(e.dict_find("c")->type() == lazy_entry::string_t); + TEST_CHECK(e.dict_find("c")->string_value() == std::string("bbb")); + TEST_CHECK(e.dict_find("c")->string_length() == 3); + TEST_CHECK(e.dict_find_string_value("X") == "0123456789"); + } + + // test strings with negative length-prefix + { + char b[] = "-10:foobar"; + lazy_entry e; + error_code ec; + int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); + TEST_CHECK(ret != 0); + printf("%s\n", print_entry(e).c_str()); + TEST_CHECK(ec == error_code(bdecode_errors::expected_value + , get_bdecode_category())); + } + + // test strings with overflow length-prefix + { + char b[] = "18446744073709551615:foobar"; + lazy_entry e; + error_code ec; + int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); + TEST_CHECK(ret != 0); + printf("%s\n", print_entry(e).c_str()); + TEST_CHECK(ec == error_code(bdecode_errors::overflow + , get_bdecode_category())); + } + + + // test integers that don't fit in 64 bits + { + char b[] = "i18446744073709551615e"; + lazy_entry e; + error_code ec; + int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); + TEST_CHECK(ret == 0); + printf("%s\n", print_entry(e).c_str()); + // the lazy aspect makes this overflow when asking for + // the value. turning it to zero. + TEST_CHECK(e.int_value() == 0); + } + + // test integers that just exactly fit in 64 bits + { + char b[] = "i9223372036854775807e"; + lazy_entry e; + error_code ec; + int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); + TEST_CHECK(ret == 0); + printf("%s\n", print_entry(e).c_str()); + TEST_CHECK(e.int_value() == 9223372036854775807LL); + } + + // test integers that just exactly fit in 64 bits + { + char b[] = "i-9223372036854775807e"; + lazy_entry e; + error_code ec; + int ret = lazy_bdecode(b, b + sizeof(b)-1, e, ec); + TEST_CHECK(ret == 0); + printf("%s\n", print_entry(e).c_str()); + TEST_CHECK(e.int_value() == -9223372036854775807LL); } // test invalid encoding