[EXPORTER] handling of invalid ports in UrlParser (open-telemetry#3142)

marcalff · Nov 14, 2024 · 149d29c · 149d29c
1 parent c81a3d4
commit 149d29c
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 5 deletions.
diff --git a/ext/include/opentelemetry/ext/http/common/url_parser.h b/ext/include/opentelemetry/ext/http/common/url_parser.h
@@ -3,8 +3,9 @@
 
 #pragma once
 
-#include <stdlib.h>
+#include <cerrno>
 #include <cstdint>
+#include <cstdlib>
 #include <string>
 
 #include "opentelemetry/version.h"
@@ -88,8 +89,11 @@ class UrlParser
       path_ = std::string("/");  // use default path
       if (is_port)
       {
-        port_ = static_cast<uint16_t>(
-            std::stoi(std::string(url_.begin() + cpos, url_.begin() + url_.length())));
+        std::string port_str(
+            url_.begin() + static_cast<std::string::difference_type>(cpos),
+            url_.begin() + static_cast<std::string::difference_type>(url_.length()));
+
+        port_ = GetPort(port_str);
       }
       else
       {
@@ -99,8 +103,9 @@ class UrlParser
     }
     if (is_port)
     {
-      port_ =
-          static_cast<uint16_t>(std::stoi(std::string(url_.begin() + cpos, url_.begin() + pos)));
+      std::string port_str(url_.begin() + static_cast<std::string::difference_type>(cpos),
+                           url_.begin() + static_cast<std::string::difference_type>(pos));
+      port_ = GetPort(port_str);
     }
     else
     {
@@ -164,6 +169,20 @@ class UrlParser
 
     return std::string::npos;
   }
+
+  std::uint16_t GetPort(const std::string &s)
+  {
+    char *e   = nullptr;
+    errno     = 0;
+    auto port = std::strtol(s.c_str(), &e, 10);
+    if (e == s.c_str() || e != s.c_str() + s.size() || errno == ERANGE || port < 0 || port > 65535)
+    {
+      success_ = false;
+      return 0;
+    }
+
+    return static_cast<uint16_t>(port);
+  }
 };
 
 class UrlDecoder

diff --git a/ext/test/http/url_parser_test.cc b/ext/test/http/url_parser_test.cc
@@ -179,6 +179,41 @@ TEST(UrlParserTests, BasicTests)
         {"path", "/path1@bbb/path2"},
         {"query", "q1=a1&q2=a2"},
         {"success", "true"}}},
+      {"https://https://example.com/some/path",
+       {{"host", "https"},
+        {"port", "0"},
+        {"scheme", "https"},
+        {"path", "//example.com/some/path"},
+        {"query", ""},
+        {"success", "false"}}},
+      {"https://example.com:-1/some/path",
+       {{"host", "example.com"},
+        {"port", "0"},
+        {"scheme", "https"},
+        {"path", "/some/path"},
+        {"query", ""},
+        {"success", "false"}}},
+      {"https://example.com:65536/some/path",
+       {{"host", "example.com"},
+        {"port", "0"},
+        {"scheme", "https"},
+        {"path", "/some/path"},
+        {"query", ""},
+        {"success", "false"}}},
+      {"https://example.com:80a/some/path",
+       {{"host", "example.com"},
+        {"port", "0"},
+        {"scheme", "https"},
+        {"path", "/some/path"},
+        {"query", ""},
+        {"success", "false"}}},
+      {"https://example.com:18446744073709551616/some/path",
+       {{"host", "example.com"},
+        {"port", "0"},
+        {"scheme", "https"},
+        {"path", "/some/path"},
+        {"query", ""},
+        {"success", "false"}}},
   };
   for (auto &url_map : urls_map)
   {