Project

General

Profile

Bug #2559

accesslog_append_escaped does not hex-escape properly

Added by TheJH over 3 years ago. Updated about 2 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_accesslog
Target version:
Start date:
2014-03-05
Due date:
% Done:

100%

Estimated time:
Missing in 1.5.x:
No

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;
                        }
                }
        }
-----------------------------------------------------------------------

Associated revisions

Revision 572681c9 (diff)
Added by stbuehler about 2 years ago

fix hex escape in accesslog (fixes #2559)

From: Stefan Bühler <>

git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@2992 152afb58-edef-0310-8abb-c4023f1b3aa9

Revision 2992 (diff)
Added by stbuehler about 2 years ago

fix hex escape in accesslog (fixes #2559)

From: Stefan Bühler <>

History

#1 Updated by TheJH over 3 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;
                        }
                }
        }

#2 Updated by stbuehler over 3 years ago

  • Target version set to 1.4.36

#3 Updated by stbuehler about 2 years ago

  • Description updated (diff)

#4 Updated by stbuehler about 2 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100

Applied in changeset r2992.

Also available in: Atom