ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

http_static: sanitize path before file read

Browse Source 
Romove dot segments from requested target path before start reading
file in file handler to prevent path traversal.

Type: fix

Change-Id: I3bdd3e9d7fffd33c9c8c608169c1dc73423b7078
Signed-off-by: Matus Fabian <matfabia@cisco.com>
This commit is contained in:
Matus Fabian
2024-05-28 13:39:13 +02:00
committed by Florin Coras
parent a93c85a579
commit 5409d33002
4 changed files with 121 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
extras/hs-test/http_test.go
View File

@ -16,10 +16,12 @@ func init() {
registerNoTopoTests(NginxHttp3Test, NginxAsServerTest,
NginxPerfCpsTest, NginxPerfRpsTest, NginxPerfWrkTest, HeaderServerTest,
HttpStaticMovedTest, HttpStaticNotFoundTest, HttpCliMethodNotAllowedTest,
HttpCliBadRequestTest)
HttpCliBadRequestTest, HttpStaticPathTraversalTest)
registerNoTopoSoloTests(HttpStaticPromTest)
}
const wwwRootPath = "/tmp/www_root"
func HttpTpsTest(s *NsSuite) {
iface := s.getInterfaceByName(clientInterface)
client_ip := iface.ip4AddressString()
@ -108,21 +110,31 @@ func HttpStaticPromTest(s *NoTopoSuite) {
s.assertNil(err)
}
func HttpStaticPathTraversalTest(s *NoTopoSuite) {
vpp := s.getContainerByName("vpp").vppInstance
vpp.container.exec("mkdir -p " + wwwRootPath)
vpp.container.exec("mkdir -p " + "/tmp/secret_folder")
vpp.container.createFile("/tmp/secret_folder/secret_file.txt", "secret")
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
s.log(vpp.vppctl("http static server www-root " + wwwRootPath + " uri tcp://" + serverAddress + "/80 debug"))
client := newHttpClient()
req, err := http.NewRequest("GET", "http://"+serverAddress+":80/../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.assertEqual(404, resp.StatusCode)
}
func HttpStaticMovedTest(s *NoTopoSuite) {
vpp := s.getContainerByName("vpp").vppInstance
vpp.container.exec("mkdir -p /tmp/tmp.aaa")
vpp.container.createFile("/tmp/tmp.aaa/index.html", "<http><body><p>Hello</p></body></http>")
vpp.container.exec("mkdir -p " + wwwRootPath + "/tmp.aaa")
vpp.container.createFile(wwwRootPath+"/tmp.aaa/index.html", "<http><body><p>Hello</p></body></http>")
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
s.log(vpp.vppctl("http static server www-root /tmp uri tcp://" + serverAddress + "/80 debug"))
s.log(vpp.vppctl("http static server www-root " + wwwRootPath + " uri tcp://" + serverAddress + "/80 debug"))
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
client := &http.Client{
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
Transport: transport,
}
client := newHttpClient()
req, err := http.NewRequest("GET", "http://"+serverAddress+":80/tmp.aaa", nil)
s.assertNil(err, fmt.Sprint(err))
resp, err := client.Do(req)
@ -134,12 +146,11 @@ func HttpStaticMovedTest(s *NoTopoSuite) {
func HttpStaticNotFoundTest(s *NoTopoSuite) {
vpp := s.getContainerByName("vpp").vppInstance
vpp.container.exec("mkdir -p " + wwwRootPath)
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
s.log(vpp.vppctl("http static server www-root /tmp uri tcp://" + serverAddress + "/80 debug"))
s.log(vpp.vppctl("http static server www-root " + wwwRootPath + " uri tcp://" + serverAddress + "/80 debug"))
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
client := &http.Client{Transport: transport}
client := newHttpClient()
req, err := http.NewRequest("GET", "http://"+serverAddress+":80/notfound.html", nil)
s.assertNil(err, fmt.Sprint(err))
resp, err := client.Do(req)
@ -153,9 +164,7 @@ func HttpCliMethodNotAllowedTest(s *NoTopoSuite) {
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
vpp.vppctl("http cli server")
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
client := &http.Client{Transport: transport}
client := newHttpClient()
req, err := http.NewRequest("POST", "http://"+serverAddress+":80/test", nil)
s.assertNil(err, fmt.Sprint(err))
resp, err := client.Do(req)
@ -171,9 +180,7 @@ func HttpCliBadRequestTest(s *NoTopoSuite) {
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
vpp.vppctl("http cli server")
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
client := &http.Client{Transport: transport}
client := newHttpClient()
req, err := http.NewRequest("GET", "http://"+serverAddress+":80", nil)
s.assertNil(err, fmt.Sprint(err))
resp, err := client.Do(req)
@ -187,9 +194,7 @@ func HeaderServerTest(s *NoTopoSuite) {
serverAddress := s.getInterfaceByName(tapInterfaceName).peer.ip4AddressString()
vpp.vppctl("http cli server")
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
client := &http.Client{Transport: transport}
client := newHttpClient()
req, err := http.NewRequest("GET", "http://"+serverAddress+":80/show/version", nil)
s.assertNil(err, fmt.Sprint(err))
resp, err := client.Do(req)

16
extras/hs-test/utils.go
View File

@ -3,8 +3,10 @@ package main
import (
"fmt"
"io"
"net/http"
"os"
"strings"
"time"
)
const networkTopologyDir string = "topo-network/"
@ -78,3 +80,17 @@ func (s *Stanza) saveToFile(fileName string) error {
_, err = io.Copy(fo, strings.NewReader(s.content))
return err
}
// newHttpClient creates [http.Client] with disabled proxy and redirects, it also sets timeout to 30seconds.
func newHttpClient() *http.Client {
transport := http.DefaultTransport
transport.(*http.Transport).Proxy = nil
transport.(*http.Transport).DisableKeepAlives = true
client := &http.Client{
Transport: transport,
Timeout: time.Second * 30,
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
}}
return client
}

68
src/plugins/http/http.h
View File

@ -277,6 +277,74 @@ http_state_is_tx_valid (http_conn_t *hc)
state == HTTP_STATE_WAIT_APP_METHOD);
}
/**
* Remove dot segments from path (RFC3986 section 5.2.4)
*
* @param path Path to sanitize.
*
* @return New vector with sanitized path.
*
* The caller is always responsible to free the returned vector.
*/
always_inline u8 *
http_path_remove_dot_segments (u8 *path)
{
u32 *segments = 0, *segments_len = 0, segment_len;
u8 *new_path = 0;
int i, ii;
if (!path)
return vec_new (u8, 0);
segments = vec_new (u32, 1);
/* first segment */
segments[0] = 0;
/* find all segments */
for (i = 1; i < (vec_len (path) - 1); i++)
{
if (path[i] == '/')
vec_add1 (segments, i + 1);
}
/* dummy tail */
vec_add1 (segments, vec_len (path));
/* scan all segments for "." and ".." */
segments_len = vec_new (u32, vec_len (segments) - 1);
for (i = 0; i < vec_len (segments_len); i++)
{
segment_len = segments[i + 1] - segments[i];
if (segment_len == 2 && path[segments[i]] == '.')
segment_len = 0;
else if (segment_len == 3 && path[segments[i]] == '.' &&
path[segments[i] + 1] == '.')
{
segment_len = 0;
/* remove parent (if any) */
for (ii = i - 1; ii >= 0; ii--)
{
if (segments_len[ii])
{
segments_len[ii] = 0;
break;
}
}
}
segments_len[i] = segment_len;
}
/* we might end with empty path, so return at least empty vector */
new_path = vec_new (u8, 0);
/* append all valid segments */
for (i = 0; i < vec_len (segments_len); i++)
{
if (segments_len[i])
vec_add (new_path, path + segments[i], segments_len[i]);
}
vec_free (segments);
vec_free (segments_len);
return new_path;
}
#endif /* SRC_PLUGINS_HTTP_HTTP_H_ */
/*

11
src/plugins/http_static/static_server.c
View File

@ -357,7 +357,7 @@ try_file_handler (hss_main_t *hsm, hss_session_t *hs, http_req_method_t rt,
u8 *request)
{
http_status_code_t sc = HTTP_STATUS_OK;
u8 *path;
u8 *path, *sanitized_path;
u32 ce_index;
http_content_type_t type;
@ -367,6 +367,9 @@ try_file_handler (hss_main_t *hsm, hss_session_t *hs, http_req_method_t rt,
type = content_type_from_request (request);
/* Remove dot segments to prevent path traversal */
sanitized_path = http_path_remove_dot_segments (request);
/*
* Construct the file to open
* Browsers are capable of sporadically including a leading '/'
@ -374,9 +377,9 @@ try_file_handler (hss_main_t *hsm, hss_session_t *hs, http_req_method_t rt,
if (!request)
path = format (0, "%s%c", hsm->www_root, 0);
else if (request[0] == '/')
path = format (0, "%s%s%c", hsm->www_root, request, 0);
path = format (0, "%s%s%c", hsm->www_root, sanitized_path, 0);
else
path = format (0, "%s/%s%c", hsm->www_root, request, 0);
path = format (0, "%s/%s%c", hsm->www_root, sanitized_path, 0);
if (hsm->debug_level > 0)
clib_warning ("%s '%s'", (rt == HTTP_REQ_GET) ? "GET" : "POST", path);
@ -419,7 +422,7 @@ try_file_handler (hss_main_t *hsm, hss_session_t *hs, http_req_method_t rt,
hs->cache_pool_index = ce_index;
done:
vec_free (sanitized_path);
hs->content_type = type;
start_send_data (hs, sc);
if (!hs->data)