Code Review / vpp.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | review | tree
raw | patch | inline | side by side (parent: 58b6c4e)
http_static: squash subsequent forward slashes in request target path 82/42382/6
authorSemir Sionek <[email protected]>
Fri, 21 Feb 2025 14:09:29 +0000 (09:09 -0500)
committerFlorin Coras <[email protected]>
Tue, 25 Feb 2025 18:03:33 +0000 (18:03 +0000)
In the file handler, squash groups of forward slashes during path
sanitation to minify the risk of running out of memory.

Type: fix

Change-Id: Ic29d691f876b891ff588157851334162b4e3c5e3
Signed-off-by: Semir Sionek <[email protected]>
extras/hs-test/http_test.go patch | blob | history
src/plugins/http/http.h patch | blob | history
src/plugins/http/http_plugin.rst patch | blob | history
src/plugins/http_static/static_server.c patch | blob | history

diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index b143e55..99c0a05 100644 (file)
--- a/extras/hs-test/http_test.go
+++ b/extras/hs-test/http_test.go
@@ -30,7 +30,7 @@ func init() {
                HttpInvalidRequestLineTest, HttpMethodNotImplementedTest, HttpInvalidHeadersTest,
                HttpContentLengthTest, HttpStaticBuildInUrlGetIfListTest, HttpStaticBuildInUrlGetVersionTest,
                HttpStaticMacTimeTest, HttpStaticBuildInUrlGetVersionVerboseTest, HttpVersionNotSupportedTest,
-               HttpInvalidContentLengthTest, HttpInvalidTargetSyntaxTest, HttpStaticPathTraversalTest, HttpUriDecodeTest,
+               HttpInvalidContentLengthTest, HttpInvalidTargetSyntaxTest, HttpStaticPathSanitizationTest, HttpUriDecodeTest,
                HttpHeadersTest, HttpStaticFileHandlerTest, HttpStaticFileHandlerDefaultMaxAgeTest, HttpClientTest,
                HttpClientErrRespTest, HttpClientPostFormTest, HttpClientGet128kbResponseTest, HttpClientGetResponseBodyTest,
                HttpClientGetNoResponseBodyTest, HttpClientPostFileTest, HttpClientPostFilePtrTest, HttpUnitTest,
@@ -865,12 +865,15 @@ func HttpStaticFileHandlerTestFunction(s *NoTopoSuite, max_age string) {
        s.AssertContains(o, "page.html")
 }
 
-func HttpStaticPathTraversalTest(s *NoTopoSuite) {
+func HttpStaticPathSanitizationTest(s *NoTopoSuite) {
        vpp := s.Containers.Vpp.VppInstance
        vpp.Container.Exec(false, "mkdir -p "+wwwRootPath)
        vpp.Container.Exec(false, "mkdir -p "+"/tmp/secret_folder")
        err := vpp.Container.CreateFile("/tmp/secret_folder/secret_file.txt", "secret")
        s.AssertNil(err, fmt.Sprint(err))
+       indexContent := "<html><body>index</body></html>"
+       err = vpp.Container.CreateFile(wwwRootPath+"/index.html", indexContent)
+       s.AssertNil(err, fmt.Sprint(err))
        serverAddress := s.VppAddr()
        s.Log(vpp.Vppctl("http static server www-root " + wwwRootPath + " uri tcp://" + serverAddress + "/80 debug"))
 
@@ -885,6 +888,26 @@ func HttpStaticPathTraversalTest(s *NoTopoSuite) {
        s.AssertHttpHeaderNotPresent(resp, "Content-Type")
        s.AssertHttpHeaderNotPresent(resp, "Cache-Control")
        s.AssertHttpContentLength(resp, int64(0))
+
+       req, err = http.NewRequest("GET", "http://"+serverAddress+":80//////fake/directory///../././//../../secret_folder/secret_file.txt", nil)
+       s.AssertNil(err, fmt.Sprint(err))
+       resp, err = client.Do(req)
+       s.AssertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.Log(DumpHttpResp(resp, true))
+       s.AssertHttpStatus(resp, 404)
+       s.AssertHttpHeaderNotPresent(resp, "Content-Type")
+       s.AssertHttpHeaderNotPresent(resp, "Cache-Control")
+       s.AssertHttpContentLength(resp, int64(0))
+
+       req, err = http.NewRequest("GET", "http://"+serverAddress+":80/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////", nil)
+       s.AssertNil(err, fmt.Sprint(err))
+       resp, err = client.Do(req)
+       s.AssertNil(err, fmt.Sprint(err))
+       defer resp.Body.Close()
+       s.Log(DumpHttpResp(resp, true))
+       s.AssertHttpStatus(resp, 301)
+       s.AssertHttpHeaderWithValue(resp, "Location", "http://"+serverAddress+"/index.html")
 }
 
 func HttpStaticMovedTest(s *NoTopoSuite) {
diff --git a/src/plugins/http/http.h b/src/plugins/http/http.h
index 0245c6e..593472a 100644 (file)
--- a/src/plugins/http/http.h
+++ b/src/plugins/http/http.h
@@ -498,7 +498,8 @@ http_percent_decode (u8 *src, u32 len)
 }
 
 /**
- * Remove dot segments from path (RFC3986 section 5.2.4)
+ * Sanitize HTTP path by squashing repeating slashes and removing
+ * dot segments from path (RFC3986 section 5.2.4)
  *
  * @param path Path to sanitize.
  *
@@ -507,18 +508,18 @@ http_percent_decode (u8 *src, u32 len)
  * The caller is always responsible to free the returned vector.
  */
 always_inline u8 *
-http_path_remove_dot_segments (u8 *path)
+http_path_sanitize (u8 *path)
 {
   u32 *segments = 0, *segments_len = 0, segment_len;
   u8 *new_path = 0;
   int i, ii;
 
-  if (!path)
+  if (!path || vec_len (path) == 0)
     return vec_new (u8, 0);
 
   segments = vec_new (u32, 1);
   /* first segment */
-  segments[0] = 0;
+  segments[0] = (path[0] == '/' ? 1 : 0);
   /* find all segments */
   for (i = 1; i < (vec_len (path) - 1); i++)
     {
@@ -533,7 +534,8 @@ http_path_remove_dot_segments (u8 *path)
   for (i = 0; i < vec_len (segments_len); i++)
     {
       segment_len = segments[i + 1] - segments[i];
-      if (segment_len == 2 && path[segments[i]] == '.')
+      /* aside from dots, skip empty segments (double slashes) */
+      if ((segment_len == 2 && path[segments[i]] == '.') || segment_len == 1)
        segment_len = 0;
       else if (segment_len == 3 && path[segments[i]] == '.' &&
               path[segments[i] + 1] == '.')
diff --git a/src/plugins/http/http_plugin.rst b/src/plugins/http/http_plugin.rst
index 995e55e..4e799a5 100644 (file)
--- a/src/plugins/http/http_plugin.rst
+++ b/src/plugins/http/http_plugin.rst
@@ -15,7 +15,7 @@ Usage
 -----
 
 The plugin exposes following inline functions: ``http_validate_abs_path_syntax``, ``http_validate_query_syntax``,
-``http_percent_decode``, ``http_path_remove_dot_segments``, ``http_build_header_table``, ``http_get_header``,
+``http_percent_decode``, ``http_path_sanitize``, ``http_build_header_table``, ``http_get_header``,
 ``http_reset_header_table``, ``http_free_header_table``, ``http_init_headers_ctx``, ``http_add_header``,
 ``http_add_custom_header``, ``http_validate_target_syntax``, ``http_parse_authority``, ``http_serialize_authority``,
 ``http_parse_masque_host_port``, ``http_decap_udp_payload_datagram``, ``http_encap_udp_payload_datagram``,
diff --git a/src/plugins/http_static/static_server.c b/src/plugins/http_static/static_server.c
index 0744168..fb0878c 100644 (file)
--- a/src/plugins/http_static/static_server.c
+++ b/src/plugins/http_static/static_server.c
@@ -406,8 +406,8 @@ try_file_handler (hss_main_t *hsm, hss_session_t *hs, http_req_method_t rt,
   if (!hsm->www_root)
     return -1;
 
-  /* Remove dot segments to prevent path traversal */
-  sanitized_path = http_path_remove_dot_segments (target);
+  /* Sanitize received path */
+  sanitized_path = http_path_sanitize (target);
 
   /*
    * Construct the file to open
Vector Packet Processing