Curb maximum size of headers
Hrvoje Niksic <[email protected]>
| Newsgroups | gmane.comp.web.wget.patches |
|---|---|
| Message-ID | <[email protected]> |
When talking HTTP, (CVS) Wget slurps all the HTTP headers in memory and analyzes them, the assumption being that the HTTP headers are fairly small. While that is true enough, it also leaves Wget users vulnerable to malicious servers causing Wget to spend all available memory. A similar problem is present in the "read line" code, used when talking to FTP servers. This patch imposes IMHO reasonable, yet safe, limits for reading server responses into memory. 2005-03-15 Hrvoje Niksic <[email protected]> * http.c (read_http_response_head): Limit the response size to 64k bytes. * retr.c (fd_read_hunk): Accept a MAXSIZE argument that limits the number of bytes the function is allowed to allocate. (fd_read_line): Limit the line to 4096 bytes. Index: src/http.c =================================================================== RCS file: /pack/anoncvs/wget/src/http.c,v retrieving revision 1.149 diff -u -r1.149 http.c --- src/http.c 2005/03/06 22:53:02 1.149 +++ src/http.c 2005/03/15 17:43:11 @@ -432,6 +432,13 @@ return NULL; } +/* The maximum size of a single HTTP response we care to read. This + is not meant to impose an arbitrary limit, but to protect the user + from Wget slurping up available memory upon encountering malicious + or buggy server output. Define it to 0 to remove the limit. */ + +#define HTTP_RESPONSE_MAX_SIZE 65536 + /* Read the HTTP request head from FD and return it. The error conditions are the same as with fd_read_hunk. @@ -443,7 +450,8 @@ static char * read_http_response_head (int fd) { - return fd_read_hunk (fd, response_head_terminator, 512); + return fd_read_hunk (fd, response_head_terminator, 512, + HTTP_RESPONSE_MAX_SIZE); } struct response { Index: src/retr.c =================================================================== RCS file: /pack/anoncvs/wget/src/retr.c,v retrieving revision 1.88 diff -u -r1.88 retr.c --- src/retr.c 2005/03/04 19:34:31 1.88 +++ src/retr.c 2005/03/15 17:43:12 @@ -375,18 +375,23 @@ a read. If the read returns a different amount of data, the process is retried until all data arrives safely. - BUFSIZE is the size of the initial buffer expected to read all the - data in the typical case. + SIZEHINT is the buffer size sufficient to hold all the data in the + typical case (it is used as the initial buffer size). MAXSIZE is + the maximum amount of memory this function is allowed to allocate, + or 0 if no upper limit is to be enforced. This function should be used as a building block for other functions -- see fd_read_line as a simple example. */ char * -fd_read_hunk (int fd, hunk_terminator_t hunk_terminator, int bufsize) +fd_read_hunk (int fd, hunk_terminator_t terminator, long sizehint, long maxsize) { + long bufsize = sizehint; char *hunk = xmalloc (bufsize); int tail = 0; /* tail position in HUNK */ + assert (maxsize >= bufsize); + while (1) { const char *end; @@ -400,7 +405,7 @@ xfree (hunk); return NULL; } - end = hunk_terminator (hunk, tail, pklen); + end = terminator (hunk, tail, pklen); if (end) { /* The data contains the terminator: we'll drain the data up @@ -458,7 +463,17 @@ if (tail == bufsize - 1) { + /* Double the buffer size, but refuse to allocate more than + MAXSIZE bytes. */ + if (maxsize && bufsize >= maxsize) + { + xfree (hunk); + errno = ENOMEM; + return NULL; + } bufsize <<= 1; + if (maxsize && bufsize > maxsize) + bufsize = maxsize; hunk = xrealloc (hunk, bufsize); } } @@ -474,8 +489,14 @@ return NULL; } +/* The maximum size of the single line we agree to accept. This is + not meant to impose an arbitrary limit, but to protect the user + from Wget slurping up available memory upon encountering malicious + or buggy server output. Define it to 0 to remove the limit. */ +#define FD_READ_LINE_MAX 4096 + /* Read one line from FD and return it. The line is allocated using - malloc. + malloc, but is never larger than FD_READ_LINE_MAX. If an error occurs, or if no data can be read, NULL is returned. In the former case errno indicates the error condition, and in the @@ -484,7 +505,7 @@ char * fd_read_line (int fd) { - return fd_read_hunk (fd, line_terminator, 128); + return fd_read_hunk (fd, line_terminator, 128, FD_READ_LINE_MAX); } /* Return a printed representation of the download rate, as Index: src/retr.h =================================================================== RCS file: /pack/anoncvs/wget/src/retr.h,v retrieving revision 1.20 diff -u -r1.20 retr.h --- src/retr.h 2005/02/23 22:21:03 1.20 +++ src/retr.h 2005/03/15 17:43:12 @@ -41,7 +41,7 @@ typedef const char *(*hunk_terminator_t) PARAMS ((const char *, int, int)); -char *fd_read_hunk PARAMS ((int, hunk_terminator_t, int)); +char *fd_read_hunk PARAMS ((int, hunk_terminator_t, long, long)); char *fd_read_line PARAMS ((int)); uerr_t retrieve_url PARAMS ((const char *, char **, char **,