diff options
| -rw-r--r-- | extras/hs-test/http_test.go | 27 | ||||
| -rw-r--r-- | src/plugins/http/http.h | 12 | ||||
| -rw-r--r-- | src/plugins/http/http_plugin.rst | 2 | ||||
| -rw-r--r-- | src/plugins/http_static/static_server.c | 4 |
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 |