Skip to content

Commit

Permalink
strtoofft: reduce integer overflow risks globally
Browse files Browse the repository at this point in the history
... make sure we bail out on overflows.

Reported-by: Brian Carpenter
  • Loading branch information
bagder committed Aug 12, 2017
1 parent 6bde13a commit 82e1d0f
Show file tree
Hide file tree
Showing 13 changed files with 196 additions and 131 deletions.
23 changes: 15 additions & 8 deletions lib/cookie.c
Original file line number Diff line number Diff line change
Expand Up @@ -593,14 +593,20 @@ Curl_cookie_add(struct Curl_easy *data,
} while(semiptr);

if(co->maxage) {
co->expires =
curlx_strtoofft((*co->maxage=='\"')?
&co->maxage[1]:&co->maxage[0], NULL, 10);
if(CURL_OFF_T_MAX - now < co->expires)
/* avoid overflow */
CURLofft offt;
offt = curlx_strtoofft((*co->maxage=='\"')?
&co->maxage[1]:&co->maxage[0], NULL, 10,
&co->expires);
if(offt == CURL_OFFT_FLOW)
/* overflow, used max value */
co->expires = CURL_OFF_T_MAX;
else
co->expires += now;
else if(!offt) {
if(CURL_OFF_T_MAX - now < co->expires)
/* would overflow */
co->expires = CURL_OFF_T_MAX;
else
co->expires += now;
}
}
else if(co->expirestr) {
/* Note that if the date couldn't get parsed for whatever reason,
Expand Down Expand Up @@ -753,7 +759,8 @@ Curl_cookie_add(struct Curl_easy *data,
co->secure = strcasecompare(ptr, "TRUE")?TRUE:FALSE;
break;
case 4:
co->expires = curlx_strtoofft(ptr, NULL, 10);
if(curlx_strtoofft(ptr, NULL, 10, &co->expires))
badcookie = TRUE;
break;
case 5:
co->name = strdup(ptr);
Expand Down
24 changes: 13 additions & 11 deletions lib/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,26 +139,28 @@ static CURLcode file_range(struct connectdata *conn)
struct Curl_easy *data = conn->data;

if(data->state.use_range && data->state.range) {
from=curlx_strtoofft(data->state.range, &ptr, 0);
CURLofft from_t;
CURLofft to_t;
from_t = curlx_strtoofft(data->state.range, &ptr, 0, &from);
if(from_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
ptr++;
to=curlx_strtoofft(ptr, &ptr2, 0);
if(ptr == ptr2) {
/* we didn't get any digit */
to=-1;
}
if((-1 == to) && (from>=0)) {
to_t = curlx_strtoofft(ptr, &ptr2, 0, &to);
if(to_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
if((to_t == CURL_OFFT_INVAL) && !from_t) {
/* X - */
data->state.resume_from = from;
DEBUGF(infof(data, "RANGE %" CURL_FORMAT_CURL_OFF_T " to end of file\n",
from));
}
else if(from < 0) {
else if((from_t == CURL_OFFT_INVAL) && !to_t) {
/* -Y */
data->req.maxdownload = -from;
data->state.resume_from = from;
data->req.maxdownload = to;
data->state.resume_from = -to;
DEBUGF(infof(data, "RANGE the last %" CURL_FORMAT_CURL_OFF_T " bytes\n",
-from));
to));
}
else {
/* X-Y */
Expand Down
33 changes: 18 additions & 15 deletions lib/ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2260,11 +2260,13 @@ static CURLcode ftp_state_size_resp(struct connectdata *conn,
{
CURLcode result = CURLE_OK;
struct Curl_easy *data=conn->data;
curl_off_t filesize;
curl_off_t filesize = -1;
char *buf = data->state.buffer;

/* get the size from the ascii string: */
filesize = (ftpcode == 213)?curlx_strtoofft(buf+4, NULL, 0):-1;
if(ftpcode == 213)
/* ignores parsing errors, which will make the size remain unknown */
(void)curlx_strtoofft(buf+4, NULL, 0, &filesize);

if(instate == FTP_SIZE) {
#ifdef CURL_FTP_HTTPSTYLE_HEAD
Expand Down Expand Up @@ -2435,7 +2437,7 @@ static CURLcode ftp_state_get_resp(struct connectdata *conn,
/* if we have nothing but digits: */
if(bytes++) {
/* get the number! */
size = curlx_strtoofft(bytes, NULL, 0);
(void)curlx_strtoofft(bytes, NULL, 0, &size);
}
}
}
Expand Down Expand Up @@ -3466,31 +3468,32 @@ static CURLcode ftp_range(struct connectdata *conn)
{
curl_off_t from, to;
char *ptr;
char *ptr2;
struct Curl_easy *data = conn->data;
struct ftp_conn *ftpc = &conn->proto.ftpc;

if(data->state.use_range && data->state.range) {
from=curlx_strtoofft(data->state.range, &ptr, 0);
CURLofft from_t;
CURLofft to_t;
from_t = curlx_strtoofft(data->state.range, &ptr, 0, &from);
if(from_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
ptr++;
to=curlx_strtoofft(ptr, &ptr2, 0);
if(ptr == ptr2) {
/* we didn't get any digit */
to=-1;
}
if((-1 == to) && (from>=0)) {
to_t = curlx_strtoofft(ptr, NULL, 0, &to);
if(to_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
if((to_t == CURL_OFFT_INVAL) && !from_t) {
/* X - */
data->state.resume_from = from;
DEBUGF(infof(conn->data, "FTP RANGE %" CURL_FORMAT_CURL_OFF_T
" to end of file\n", from));
}
else if(from < 0) {
else if(!to_t && (from_t == CURL_OFFT_INVAL)) {
/* -Y */
data->req.maxdownload = -from;
data->state.resume_from = from;
data->req.maxdownload = to;
data->state.resume_from = -to;
DEBUGF(infof(conn->data, "FTP RANGE the last %" CURL_FORMAT_CURL_OFF_T
" bytes\n", -from));
" bytes\n", to));
}
else {
/* X-Y */
Expand Down
36 changes: 14 additions & 22 deletions lib/ftplistparser.c
Original file line number Diff line number Diff line change
Expand Up @@ -609,16 +609,18 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
char *p;
curl_off_t fsize;
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
fsize = curlx_strtoofft(finfo->b_data+parser->item_offset, &p, 10);
if(p[0] == '\0' && fsize != CURL_OFF_T_MAX &&
fsize != CURL_OFF_T_MIN) {
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->file_data->info.size = fsize;
if(!curlx_strtoofft(finfo->b_data+parser->item_offset,
&p, 10, &fsize)) {
if(p[0] == '\0' && fsize != CURL_OFF_T_MAX &&
fsize != CURL_OFF_T_MIN) {
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->file_data->info.size = fsize;
}
parser->item_length = 0;
parser->item_offset = 0;
parser->state.UNIX.main = PL_UNIX_TIME;
parser->state.UNIX.sub.time = PL_UNIX_TIME_PREPART1;
}
parser->item_length = 0;
parser->item_offset = 0;
parser->state.UNIX.main = PL_UNIX_TIME;
parser->state.UNIX.sub.time = PL_UNIX_TIME_PREPART1;
}
else if(!ISDIGIT(c)) {
PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
Expand Down Expand Up @@ -935,19 +937,9 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
}
else {
char *endptr;
finfo->size = curlx_strtoofft(finfo->b_data +
parser->item_offset,
&endptr, 10);
if(!*endptr) {
if(finfo->size == CURL_OFF_T_MAX ||
finfo->size == CURL_OFF_T_MIN) {
if(errno == ERANGE) {
PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
return bufflen;
}
}
}
else {
if(curlx_strtoofft(finfo->b_data +
parser->item_offset,
&endptr, 10, &finfo->size)) {
PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
return bufflen;
}
Expand Down
56 changes: 30 additions & 26 deletions lib/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -3486,28 +3486,32 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
/* Check for Content-Length: header lines to get size */
if(!k->ignorecl && !data->set.ignorecl &&
checkprefix("Content-Length:", k->p)) {
curl_off_t contentlength = curlx_strtoofft(k->p+15, NULL, 10);
if(data->set.max_filesize &&
contentlength > data->set.max_filesize) {
failf(data, "Maximum file size exceeded");
return CURLE_FILESIZE_EXCEEDED;
}
if(contentlength >= 0) {
k->size = contentlength;
k->maxdownload = k->size;
/* we set the progress download size already at this point
just to make it easier for apps/callbacks to extract this
info as soon as possible */
Curl_pgrsSetDownloadSize(data, k->size);
}
else {
/* Negative Content-Length is really odd, and we know it
happens for example when older Apache servers send large
files */
streamclose(conn, "negative content-length");
infof(data, "Negative content-length: %" CURL_FORMAT_CURL_OFF_T
", closing after transfer\n", contentlength);
curl_off_t contentlength;
if(!curlx_strtoofft(k->p+15, NULL, 10, &contentlength)) {
if(data->set.max_filesize &&
contentlength > data->set.max_filesize) {
failf(data, "Maximum file size exceeded");
return CURLE_FILESIZE_EXCEEDED;
}
if(contentlength >= 0) {
k->size = contentlength;
k->maxdownload = k->size;
/* we set the progress download size already at this point
just to make it easier for apps/callbacks to extract this
info as soon as possible */
Curl_pgrsSetDownloadSize(data, k->size);
}
else {
/* Negative Content-Length is really odd, and we know it
happens for example when older Apache servers send large
files */
streamclose(conn, "negative content-length");
infof(data, "Negative content-length: %" CURL_FORMAT_CURL_OFF_T
", closing after transfer\n", contentlength);
}
}
else
infof(data, "Illegal Content-Length: header\n");
}
/* check for Content-Type: header lines to get the MIME-type */
else if(checkprefix("Content-Type:", k->p)) {
Expand Down Expand Up @@ -3682,11 +3686,11 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,

/* if it truly stopped on a digit */
if(ISDIGIT(*ptr)) {
k->offset = curlx_strtoofft(ptr, NULL, 10);

if(data->state.resume_from == k->offset)
/* we asked for a resume and we got it */
k->content_range = TRUE;
if(!curlx_strtoofft(ptr, NULL, 10, &k->offset)) {
if(data->state.resume_from == k->offset)
/* we asked for a resume and we got it */
k->content_range = TRUE;
}
}
else
data->state.resume_from = 0; /* get everything */
Expand Down
6 changes: 2 additions & 4 deletions lib/http_chunks.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 1998 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand Down Expand Up @@ -158,9 +158,7 @@ CHUNKcode Curl_httpchunk_read(struct connectdata *conn,
return CHUNKE_ILLEGAL_HEX;
}

ch->datasize=curlx_strtoofft(ch->hexbuffer, &endptr, 16);
if((ch->datasize == CURL_OFF_T_MAX) && (errno == ERANGE))
/* overflow is an error */
if(curlx_strtoofft(ch->hexbuffer, &endptr, 16, &ch->datasize))
return CHUNKE_ILLEGAL_HEX;
ch->state = CHUNK_LF; /* now wait for the CRLF */
}
Expand Down
4 changes: 2 additions & 2 deletions lib/http_proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,8 @@ static CURLcode CONNECT(struct connectdata *conn,
k->httpcode);
}
else {
s->cl = curlx_strtoofft(s->line_start +
strlen("Content-Length:"), NULL, 10);
(void)curlx_strtoofft(s->line_start +
strlen("Content-Length:"), NULL, 10, &s->cl);
}
}
else if(Curl_compareheader(s->line_start, "Connection:", "close"))
Expand Down
9 changes: 5 additions & 4 deletions lib/imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1070,10 +1070,11 @@ static CURLcode imap_state_fetch_resp(struct connectdata *conn, int imapcode,

if(*ptr == '{') {
char *endptr;
size = curlx_strtoofft(ptr + 1, &endptr, 10);
if(endptr - ptr > 1 && endptr[0] == '}' &&
endptr[1] == '\r' && endptr[2] == '\0')
parsed = TRUE;
if(!curlx_strtoofft(ptr + 1, &endptr, 10, &size)) {
if(endptr - ptr > 1 && endptr[0] == '}' &&
endptr[1] == '\r' && endptr[2] == '\0')
parsed = TRUE;
}
}

if(parsed) {
Expand Down
17 changes: 12 additions & 5 deletions lib/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -2233,18 +2233,25 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
curl_off_t from, to;
char *ptr;
char *ptr2;
CURLofft to_t;
CURLofft from_t;

from=curlx_strtoofft(conn->data->state.range, &ptr, 0);
from_t = curlx_strtoofft(conn->data->state.range, &ptr, 0, &from);
if(from_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
ptr++;
to=curlx_strtoofft(ptr, &ptr2, 0);
if((ptr == ptr2) /* no "to" value given */
to_t = curlx_strtoofft(ptr, &ptr2, 0, &to);
if(to_t == CURL_OFFT_FLOW)
return CURLE_RANGE_ERROR;
if((to_t == CURL_OFFT_INVAL) /* no "to" value given */
|| (to >= size)) {
to = size - 1;
}
if(from < 0) {
if(from_t) {
/* from is relative to end of file */
from += size;
from = size - to;
to = size - 1;
}
if(from > size) {
failf(data, "Offset (%"
Expand Down
Loading

0 comments on commit 82e1d0f

Please sign in to comment.