Actions
Bug #2559
closedaccesslog_append_escaped does not hex-escape properly
ASK QUESTIONS IN Forums:
Description
When accesslog_append_escaped hex-escapes a character, it incorrectly uses the type "char" instead of "unsigned char", meaning that on typical systems, the offset relative to 'A' or '0' can become negative. Not a very big issue though because the resulting characters are still in the allowed range – not dangerous, just weird. :D
Here is a simple PoC:
$ echo -ne 'GET /blah HTTP/1.1\r\nHost: blah\r\nUser-Agent: \xad\xbe\r\n\r\n' | nc ::1 80 HTTP/1.1 404 Not Found Content-Type: text/html Content-Length: 345 Date: Wed, 05 Mar 2014 04:52:08 GMT Server: lighttpd/1.4.34
Then look into the access log:
::1 blah - [05/Mar/2014:05:52:08 +0100] "GET /blah HTTP/1.1" 404 345 "-" "\x+-\x,."
The fix is pretty straightforward, just cast the input chars to unsigned before splitting them into two parts with / and %:
----------------------------------------------------------------------- diff -rpNU 8 lighttpd-1.4.34/src/mod_accesslog.c lighttpd-1.4.34-logescape/src/mod_accesslog.c --- lighttpd-1.4.34/src/mod_accesslog.c 2013-11-13 18:30:52.000000000 +0100 +++ lighttpd-1.4.34-logescape/src/mod_accesslog.c 2014-03-05 06:01:34.683818499 +0100 @@ -199,19 +199,19 @@ static void accesslog_append_escaped(buf BUFFER_APPEND_STRING_CONST(dest, "\\t"); break; case '\v': BUFFER_APPEND_STRING_CONST(dest, "\\v"); break; default: { /* non printable char => \xHH */ char hh[5] = {'\\','x',0,0,0}; - char h = c / 16; + char h = ((unsigned char)c) / 16; hh[2] = (h > 9) ? (h - 10 + 'A') : (h + '0'); - h = c % 16; + h = ((unsigned char)c) % 16; hh[3] = (h > 9) ? (h - 10 + 'A') : (h + '0'); buffer_append_string_len(dest, &hh[0], 4); } break; } } } -----------------------------------------------------------------------
Updated by TheJH almost 11 years ago
Argh, stupid formatting. This was the access log entry:
::1 blah - [05/Mar/2014:05:52:08 +0100] "GET /blah HTTP/1.1" 404 345 "-" "\x+-\x,."
And here is the patch again:
diff -rpNU 8 lighttpd-1.4.34/src/mod_accesslog.c lighttpd-1.4.34-logescape/src/mod_accesslog.c --- lighttpd-1.4.34/src/mod_accesslog.c 2013-11-13 18:30:52.000000000 +0100 +++ lighttpd-1.4.34-logescape/src/mod_accesslog.c 2014-03-05 06:01:34.683818499 +0100 @@ -199,19 +199,19 @@ static void accesslog_append_escaped(buf BUFFER_APPEND_STRING_CONST(dest, "\\t"); break; case '\v': BUFFER_APPEND_STRING_CONST(dest, "\\v"); break; default: { /* non printable char => \xHH */ char hh[5] = {'\\','x',0,0,0}; - char h = c / 16; + char h = ((unsigned char)c) / 16; hh[2] = (h > 9) ? (h - 10 + 'A') : (h + '0'); - h = c % 16; + h = ((unsigned char)c) % 16; hh[3] = (h > 9) ? (h - 10 + 'A') : (h + '0'); buffer_append_string_len(dest, &hh[0], 4); } break; } } }
Updated by stbuehler over 9 years ago
- Status changed from Patch Pending to Fixed
- % Done changed from 0 to 100
Applied in changeset r2992.
Actions
Also available in: Atom