[dev] [quark] one byte stack overflow

From: Aaron Burrow <burrows_AT_charstarstar.com>
Date: Mon, 16 Jul 2018 13:35:14 -0700

quark has a 1 byte NULL stack overflow triggered by an HTTP request for a
folder path not ending with '/' and having length PATH_MAX-1. Relevant
code from http.c:http_send_response()

    if (S_ISDIR(st.st_mode)) {
        /* add / to target if not present */
        len = strlen(realtarget);
        if (len == PATH_MAX - 2) {
            return http_send_status(fd, S_REQUEST_TOO_LARGE);
        }
        if (len && realtarget[len - 1] != '/') {
            realtarget[len] = '/';
            realtarget[len + 1] = '\0';
        }
    }

The issue is mitigated by changing the second conditional to

        if (len >= PATH_MAX - 2) {

There are two ways to trigger the bug. On my system, PATH_MAX = 4096 and the
default quark config has HEADER_MAX = 4096. The bug is triggered by folder
paths of length 4095. The `-m` option lets me increase the path length
despite the HEADER_MAX constraint. Something like,

    # mkdir -p $(ruby -e 'puts "#{("/"+"X"*99)*40 + "/" + "X"*94}"')
    # ./quark -h 127.0.0.1 -p 10000 -d / -m "foo /X $(ruby -e 'puts "/" + "X"*17')"

    # curl -vvvv -X GET -H 'Host:' -H 'User-Agent:' -H 'Accept:' $(ruby -e 'puts "http://127.0.0.1:10000#{"/" + "X"*83 + ("/"+"X"*99)*39 + "/" + "X"*94}"')

Seventeen bytes are used for 'GET', 'HTTP/1.1' and whitespace. The longest
queryable path in a valid HTTP header is 4096-17=4079 bytes. The quark map
increases the path length by 16 bytes. The forward slash appending code
writes a NULL byte to realtarget[4079+16+1=4096], which is out of bounds.

Alternatively, the bug can be triggered without `-m` if the system has a
'small' PATH_MAX or the configuration has a 'large' HEADER_MAX. The sufficient
condition is,

    HEADER_MAX >= (PATH_MAX - 1) + 17

I set HEADER_MAX to 8192 (allows me to leave the request headers) and triggered
the bug like this,

    # mkdir -p $(ruby -e 'puts "#{("/"+"X"*99)*40 + "/" + "X"*94}"')
    # ./quark -h 127.0.0.1 -p 10000 -d /

    # curl -vvvv -X GET $(ruby -e 'puts "http://127.0.0.1:10000#{("/"+"X"*99)*40 + "/" + "X"*94}"')


quark version,

    $ git log -n 1
    commit 9ff3f780e1d1912b89727140df7c1529ab8c5146
    Author: Laslo Hunhold
    Date: Mon Jul 2 18:43:06 2018 +0200

        Send a relative redirection header wherever possible
        
        This makes quark much more flexible when it is run behind a network
        filter or other kind of tunnel. Only send an absolute redirection when
        we are handling vhosts.


PATH_MAX,

    $ grep PATH_MAX /usr/include/linux/limits.h
    #define PATH_MAX 4096 /* # chars in a path name including nul */


patch,


Fix one byte NULL stack overflow.

Don't append a forward slash if the length of a folder is PATH_MAX-1. This can
happen if HEADER_MAX is larger than PATH_MAX or if the `-m` option is used to
increase the path length.

---
 http.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index f0b84b1..7a801a5 100644
--- a/http.c
+++ b/http.c
_AT_@ -430,7 +430,7 @@ http_send_response(int fd, struct request *r)
        if (S_ISDIR(st.st_mode)) {
                /* add / to target if not present */
                len = strlen(realtarget);
-               if (len == PATH_MAX - 2) {
+               if (len >= PATH_MAX - 2) {
                        return http_send_status(fd, S_REQUEST_TOO_LARGE);
                }
                if (len && realtarget[len - 1] != '/') {
-- 
burrows
    

Received on Mon Jul 16 2018 - 22:35:14 CEST