fix invalid read in parse_int() in bdecode_node() and lazy_bdecode()

ChangeLog

@@ -1,3 +1,4 @@
+	* fix invalid read in parse_int in bdecoder
 	* fix issue with very long tracker- and web seed URLs
 	* don't attempt to create empty files on startup, if they already exist
 	* fix force-recheck issue (new files would not be picked up)

src/bdecode.cpp

@@ -126,10 +126,12 @@ namespace libtorrent
 	} // anonymous namespace
 
 
-	// fills in 'val' with what the string between start and the
-	// first occurance of the delimiter is interpreted as an int.
-	// return the pointer to the delimiter, or 0 if there is a
-	// parse error. val should be initialized to zero
+	// reads the string between start and end, or up to the first occurrance of
+	// 'delimiter', whichever comes first. This string is interpreted as an
+	// integer which is assigned to 'val'. If there's a non-delimiter and
+	// non-digit in the range, a parse error is reported in 'ec'. If the value
+	// cannot be represented by the variable 'val' and overflow error is reported
+	// by 'ec'.
 	char const* parse_int(char const* start, char const* end, char delimiter
 		, boost::int64_t& val, bdecode_errors::error_code_enum& ec)
 	{
@@ -155,8 +157,6 @@ namespace libtorrent
 			val += digit;
 			++start;
 		}
-		if (*start != delimiter)
-			ec = bdecode_errors::expected_colon;
 		return start;
 	}
 
@@ -618,16 +618,18 @@ namespace libtorrent
 		bdecode_token const& t = m_root_tokens[m_token_idx];
 		int size = m_root_tokens[m_token_idx + 1].offset - t.offset;
 		TORRENT_ASSERT(t.type == bdecode_token::integer);
-	
+
 		// +1 is to skip the 'i'
 		char const* ptr = m_buffer + t.offset + 1;
 		boost::int64_t val = 0;
 		bool negative = false;
 		if (*ptr == '-') negative = true;
 		bdecode_errors::error_code_enum ec = bdecode_errors::no_error;
-		parse_int(ptr + negative
+		char const* end = parse_int(ptr + negative
 			, ptr + size, 'e', val, ec);
 		if (ec) return 0;
+		TORRENT_UNUSED(end);
+		TORRENT_ASSERT(end < ptr + size);
 		if (negative) val = -val;
 		return val;
 	}
@@ -844,6 +846,8 @@ namespace libtorrent
 					start = parse_int(start, end, ':', len, e);
 					if (e)
 						TORRENT_FAIL_BDECODE(e);
+					if (start == end)
+						TORRENT_FAIL_BDECODE(bdecode_errors::expected_colon);
 
 					// remaining buffer size excluding ':'
 					const ptrdiff_t buff_size = end - start - 1;

src/lazy_bdecode.cpp

@@ -131,6 +131,8 @@ namespace libtorrent
 					start = parse_int(start, end, ':', len, e);
 					if (e)
 						TORRENT_FAIL_BDECODE(e);
+					if (start == end)
+						TORRENT_FAIL_BDECODE(bdecode_errors::expected_colon);
 
 					// remaining buffer size excluding ':'
 					const ptrdiff_t buff_size = end - start - 1;
@@ -203,6 +205,8 @@ namespace libtorrent
 					start = parse_int(start, end, ':', len, e);
 					if (e)
 						TORRENT_FAIL_BDECODE(e);
+					if (start == end)
+						TORRENT_FAIL_BDECODE(bdecode_errors::expected_colon);
 
 					// remaining buffer size excluding ':'
 					const ptrdiff_t buff_size = end - start - 1;

test/test_bdecode.cpp

@@ -715,8 +715,9 @@ TORRENT_TEST(parse_int)
 {
 	char b[] = "1234567890e";
 	boost::int64_t val = 0;
-	bdecode_errors::error_code_enum ec;
+	bdecode_errors::error_code_enum ec = bdecode_errors::no_error;
 	char const* e = parse_int(b, b + sizeof(b)-1, 'e', val, ec);
+	TEST_EQUAL(ec, bdecode_errors::no_error);
 	TEST_EQUAL(val, 1234567890);
 	TEST_EQUAL(e, b + sizeof(b) - 2);
 }
@@ -759,7 +760,7 @@ TORRENT_TEST(parse_length_overflow)
 		bdecode_node e;
 		int ret = bdecode(b[i], b[i] + strlen(b[i]), e, ec);
 		TEST_EQUAL(ret, -1);
-		TEST_CHECK(ec == error_code(bdecode_errors::unexpected_eof));
+		TEST_EQUAL(ec, error_code(bdecode_errors::unexpected_eof));
 	}
 }
 
@@ -768,9 +769,9 @@ TORRENT_TEST(expected_colon_string)
 {
 	char b[] = "928";
 	boost::int64_t val = 0;
-	bdecode_errors::error_code_enum ec;
+	bdecode_errors::error_code_enum ec = bdecode_errors::no_error;
 	char const* e = parse_int(b, b + sizeof(b)-1, ':', val, ec);
-	TEST_EQUAL(ec, bdecode_errors::expected_colon);
+	TEST_EQUAL(ec, bdecode_errors::no_error);
 	TEST_EQUAL(e, b + 3);
 }
 
@@ -1253,6 +1254,21 @@ TORRENT_TEST(partial_parse4)
 	TEST_EQUAL(print_entry(e), "{ 'a': 1, 'b': 'foo', 'c': [ 1 ] }");
 }
 
+TORRENT_TEST(partial_parse_string)
+{
+	// it's important to not have a null terminator here
+	// to allow address sanitizer to trigger in case the decoder reads past the
+	// end
+	char b[] = { '5', '5'};
+
+	bdecode_node e;
+	error_code ec;
+	int pos;
+	int ret = bdecode(b, b + sizeof(b), e, ec, &pos);
+	TEST_EQUAL(ret, -1);
+	TEST_EQUAL(pos, 2);
+}
+
 // test switch_underlying_buffer
 TORRENT_TEST(switch_buffer)
 {

test/test_bencoding.cpp

@@ -587,8 +587,9 @@ TORRENT_TEST(lazy_entry)
 	{
 		char b[] = "1234567890e";
 		boost::int64_t val = 0;
-		bdecode_errors::error_code_enum ec;
+		bdecode_errors::error_code_enum ec = bdecode_errors::no_error;
 		char const* e = parse_int(b, b + sizeof(b)-1, 'e', val, ec);
+		TEST_CHECK(ec == bdecode_errors::no_error);
 		TEST_EQUAL(val, 1234567890);
 		TEST_EQUAL(e, b + sizeof(b) - 2);
 	}
@@ -615,9 +616,9 @@ TORRENT_TEST(lazy_entry)
 	{
 		char b[] = "928";
 		boost::int64_t val = 0;
-		bdecode_errors::error_code_enum ec;
+		bdecode_errors::error_code_enum ec = bdecode_errors::no_error;
 		char const* e = parse_int(b, b + sizeof(b)-1, ':', val, ec);
-		TEST_CHECK(ec == bdecode_errors::expected_colon);
+		TEST_CHECK(ec == bdecode_errors::no_error);
 		TEST_EQUAL(e, b + 3);
 	}