[Lldb-commits] [lldb] r226204 - UriParser - fixed potential buffer overrun

[Vince Harron]

Author: vharron
Date: Thu Jan 15 14:57:01 2015
New Revision: 226204

URL: http://llvm.org/viewvc/llvm-project?rev=226204&view=rev
Log:
UriParser - fixed potential buffer overrun

Switched from ::strtoul to StringConvert::ToUInt32
Changed port output parameter to be -1 if port is unspecified


Modified:
    lldb/trunk/gtest/unittest/Utility/Makefile
    lldb/trunk/gtest/unittest/Utility/UriParserTest.cpp
    lldb/trunk/source/Utility/UriParser.cpp
    lldb/trunk/source/Utility/UriParser.h

Modified: lldb/trunk/gtest/unittest/Utility/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/gtest/unittest/Utility/Makefile?rev=226204&r1=226203&r2=226204&view=diff
==============================================================================
--- lldb/trunk/gtest/unittest/Utility/Makefile (original)
+++ lldb/trunk/gtest/unittest/Utility/Makefile Thu Jan 15 14:57:01 2015
@@ -5,6 +5,7 @@ LEVEL := $(realpath $(THIS_FILE_DIR)../.
 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

Modified: lldb/trunk/gtest/unittest/Utility/UriParserTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/gtest/unittest/Utility/UriParserTest.cpp?rev=226204&r1=226203&r2=226204&view=diff
==============================================================================
--- lldb/trunk/gtest/unittest/Utility/UriParserTest.cpp (original)
+++ lldb/trunk/gtest/unittest/Utility/UriParserTest.cpp Thu Jan 15 14:57:01 2015
@@ -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
+}
+

Modified: lldb/trunk/source/Utility/UriParser.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/UriParser.cpp?rev=226204&r1=226203&r2=226204&view=diff
==============================================================================
--- lldb/trunk/source/Utility/UriParser.cpp (original)
+++ lldb/trunk/source/Utility/UriParser.cpp Thu Jan 15 14:57:01 2015
@@ -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)

Modified: lldb/trunk/source/Utility/UriParser.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/UriParser.h?rev=226204&r1=226203&r2=226204&view=diff
==============================================================================
--- lldb/trunk/source/Utility/UriParser.h (original)
+++ lldb/trunk/source/Utility/UriParser.h Thu Jan 15 14:57:01 2015
@@ -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,