aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSemir Sionek <ssionek@cisco.com>2025-02-21 09:09:29 -0500
committerFlorin Coras <florin.coras@gmail.com>2025-02-25 18:03:33 +0000
commit1cdebd8ca18bdf38af95047b1e9daf520e03030c (patch)
treef5f9dcaab67c1e0bd1f54d588abad022e5e8e1fd
parent58b6c4e6bdfba7c2a652e121c1c4e907df685780 (diff)
http_static: squash subsequent forward slashes in request target path
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 <ssionek@cisco.com>
Diffstat
-rw-r--r--extras/hs-test/http_test.go27
-rw-r--r--src/plugins/http/http.h12
-rw-r--r--src/plugins/http/http_plugin.rst2
-rw-r--r--src/plugins/http_static/static_server.c4
4 files changed, 35 insertions, 10 deletions
diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index b143e559244..99c0a05b849 100644
--- 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 0245c6e696b..593472a3591 100644
--- 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 995e55e6f0f..4e799a57668 100644
--- 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 074416873e3..fb0878c7a7e 100644
--- 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