From 72ce32e8aba16a4b593ff313bb159155d983937a Mon Sep 17 00:00:00 2001 From: hniksic Date: Sat, 19 Mar 2005 09:35:15 -0800 Subject: [PATCH] [svn] Limit the maximum amount of memory allocated by fd_read_hunk and its callers. Don't allocate more than 64k bytes on headers; don't allocate more than 4k bytes on a single line. --- src/ChangeLog | 15 +++++++++++++++ src/ftp-basic.c | 5 +---- src/http.c | 10 +++++++++- src/retr.c | 33 +++++++++++++++++++++++++++------ src/retr.h | 2 +- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 2bc2c557..67059694 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,18 @@ +2005-03-17 Hrvoje Niksic + + * ftp-basic.c (ftp_login): Don't free the string if ftp_response + returned an error status because the line didn't get allocated in + the first place. + +2005-03-15 Hrvoje Niksic + + * 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. + 2005-03-12 Hrvoje Niksic * wget.h: Include options.h after wgint has been defined. diff --git a/src/ftp-basic.c b/src/ftp-basic.c index 5960ade6..5d32aadc 100644 --- a/src/ftp-basic.c +++ b/src/ftp-basic.c @@ -123,10 +123,7 @@ ftp_login (int csock, const char *acc, const char *pass) /* Get greeting. */ err = ftp_response (csock, &respline); if (err != FTPOK) - { - xfree (respline); - return err; - } + return err; if (*respline != '2') { xfree (respline); diff --git a/src/http.c b/src/http.c index 646a031e..c77a93af 100644 --- a/src/http.c +++ b/src/http.c @@ -432,6 +432,13 @@ response_head_terminator (const char *hunk, int oldlen, int peeklen) 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 @@ response_head_terminator (const char *hunk, int oldlen, int peeklen) 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 { diff --git a/src/retr.c b/src/retr.c index ef142bce..978fb728 100644 --- a/src/retr.c +++ b/src/retr.c @@ -375,18 +375,23 @@ fd_read_body (int fd, FILE *out, wgint toread, wgint startpos, 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 @@ fd_read_hunk (int fd, hunk_terminator_t hunk_terminator, int bufsize) 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 @@ fd_read_hunk (int fd, hunk_terminator_t hunk_terminator, int bufsize) 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 @@ line_terminator (const char *hunk, int oldlen, int peeklen) 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 @@ line_terminator (const char *hunk, int oldlen, int peeklen) 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 diff --git a/src/retr.h b/src/retr.h index fa90ea95..6f896d97 100644 --- a/src/retr.h +++ b/src/retr.h @@ -41,7 +41,7 @@ int fd_read_body PARAMS ((int, FILE *, wgint, wgint, wgint *, wgint *, double *, 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 **, -- 2.39.2