From e6c5dcf5125351d084bfc6466cdfcd5d9898c0af Mon Sep 17 00:00:00 2001 From: Vince Harron Date: Thu, 15 Jan 2015 20:57:01 +0000 Subject: [PATCH] UriParser - fixed potential buffer overrun Switched from ::strtoul to StringConvert::ToUInt32 Changed port output parameter to be -1 if port is unspecified llvm-svn: 226204 --- lldb/gtest/unittest/Utility/Makefile | 1 + lldb/gtest/unittest/Utility/UriParserTest.cpp | 10 ++++++++-- lldb/source/Utility/UriParser.cpp | 21 ++++++++++++++------- lldb/source/Utility/UriParser.h | 8 ++++++++ 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lldb/gtest/unittest/Utility/Makefile b/lldb/gtest/unittest/Utility/Makefile index c35aa6c..6b444f5 100644 --- a/lldb/gtest/unittest/Utility/Makefile +++ b/lldb/gtest/unittest/Utility/Makefile @@ -5,6 +5,7 @@ LEVEL := $(realpath $(THIS_FILE_DIR)../../make) CFLAGS_EXTRAS := -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS ENABLE_THREADS := YES CXX_SOURCES := $(wildcard *.cpp) \ + $(realpath $(LEVEL)/../../source/Host/common/StringConvert.cpp) \ $(realpath $(LEVEL)/../../source/Utility/StringExtractor.cpp) \ $(realpath $(LEVEL)/../../source/Utility/UriParser.cpp) MAKE_DSYM := NO diff --git a/lldb/gtest/unittest/Utility/UriParserTest.cpp b/lldb/gtest/unittest/Utility/UriParserTest.cpp index b9db7ac..a11dd8e 100644 --- a/lldb/gtest/unittest/Utility/UriParserTest.cpp +++ b/lldb/gtest/unittest/Utility/UriParserTest.cpp @@ -57,7 +57,7 @@ public: TEST_F (UriParserTest, Minimal) { - const UriTestCase testCase("x://y", "x", "y", 0, "/"); + const UriTestCase testCase("x://y", "x", "y", -1, "/"); VALIDATE } @@ -69,7 +69,7 @@ TEST_F (UriParserTest, MinimalPort) TEST_F (UriParserTest, MinimalPath) { - const UriTestCase testCase("x://y/", "x", "y", 0, "/"); + const UriTestCase testCase("x://y/", "x", "y", -1, "/"); VALIDATE } @@ -127,3 +127,9 @@ TEST_F (UriParserTest, Empty) VALIDATE } +TEST_F (UriParserTest, PortOverflow) +{ + const UriTestCase testCase("x://y:0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/"); + VALIDATE +} + diff --git a/lldb/source/Utility/UriParser.cpp b/lldb/source/Utility/UriParser.cpp index bf1e601..1d4402f 100644 --- a/lldb/source/Utility/UriParser.cpp +++ b/lldb/source/Utility/UriParser.cpp @@ -15,6 +15,9 @@ // C++ Includes // Other libraries and framework includes // Project includes +#include "lldb/Host/StringConvert.h" + +using namespace lldb_private; //---------------------------------------------------------------------- // UriParser::Parse @@ -33,17 +36,21 @@ UriParser::Parse(const char* uri, char path_buf[2049] = {'/', 0}; bool ok = false; - if (4==sscanf(uri, "%99[^:/]://%255[^/:]:%[^/]/%2047s", scheme_buf, hostname_buf, port_buf, path_buf+1)) { ok = true; } - else if (3==sscanf(uri, "%99[^:/]://%255[^/:]:%[^/]", scheme_buf, hostname_buf, port_buf)) { ok = true; } + if (4==sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]/%2047s", scheme_buf, hostname_buf, port_buf, path_buf+1)) { ok = true; } + else if (3==sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]", scheme_buf, hostname_buf, port_buf)) { ok = true; } else if (3==sscanf(uri, "%99[^:/]://%255[^/]/%2047s", scheme_buf, hostname_buf, path_buf+1)) { ok = true; } else if (2==sscanf(uri, "%99[^:/]://%255[^/]", scheme_buf, hostname_buf)) { ok = true; } - char* end = port_buf; - int port_tmp = strtoul(port_buf, &end, 10); - if (*end != 0) + bool success = false; + int port_tmp = -1; + if (port_buf[0]) { - // there are invalid characters in port_buf - return false; + port_tmp = StringConvert::ToUInt32(port_buf, UINT32_MAX, 10, &success); + if (!success || port_tmp > 65535) + { + // there are invalid characters in port_buf + return false; + } } if (ok) diff --git a/lldb/source/Utility/UriParser.h b/lldb/source/Utility/UriParser.h index c46628d..fb129ea 100644 --- a/lldb/source/Utility/UriParser.h +++ b/lldb/source/Utility/UriParser.h @@ -20,6 +20,14 @@ class UriParser { public: + // Parses + // RETURN VALUE + // if url is valid, function returns true and + // scheme/hostname/port/path are set to the parsed values + // port it set to -1 if it is not included in the URL + // + // if the url is invalid, function returns false and + // output parameters remain unchanged static bool Parse(const char* uri, std::string& scheme, std::string& hostname, -- 2.7.4