diff options
author | Timo Teräs <timo.teras@iki.fi> | 2021-07-16 10:54:08 +0300 |
---|---|---|
committer | Timo Teräs <timo.teras@iki.fi> | 2021-07-26 14:43:35 +0300 |
commit | 36048e8fef019c5be938f8a688845b6eef1d46ab (patch) | |
tree | 0aa34f757463289276b3ccacc5395fdc5decca85 | |
parent | 41a6e4c247e68e906bea1ca7c31f0e8d3b49bc83 (diff) | |
download | apk-tools-36048e8fef019c5be938f8a688845b6eef1d46ab.tar.gz apk-tools-36048e8fef019c5be938f8a688845b6eef1d46ab.tar.bz2 apk-tools-36048e8fef019c5be938f8a688845b6eef1d46ab.tar.xz apk-tools-36048e8fef019c5be938f8a688845b6eef1d46ab.zip |
libfetch: fix range checking for http/ftp protocol parsing
Various parsing of numeric strings were not having adequate range
checking causing information leak or potential crash.
CVE-2021-36159
fixes #10749
Co-authored-by: Ariadne Conill <ariadne@dereferenced.org>
Reported-by: Samanta Navarro <ferivoz@riseup.net>
-rw-r--r-- | libfetch/common.c | 24 | ||||
-rw-r--r-- | libfetch/common.h | 12 | ||||
-rw-r--r-- | libfetch/fetch.c | 15 | ||||
-rw-r--r-- | libfetch/ftp.c | 18 | ||||
-rw-r--r-- | libfetch/http.c | 62 |
5 files changed, 70 insertions, 61 deletions
diff --git a/libfetch/common.c b/libfetch/common.c index e91b0c6..0a51c26 100644 --- a/libfetch/common.c +++ b/libfetch/common.c @@ -171,6 +171,30 @@ fetch_info(const char *fmt, ...) /*** Network-related utility functions ***************************************/ +uintmax_t +fetch_parseuint(const char *str, const char **endptr, int radix, uintmax_t max) +{ + uintmax_t val = 0, maxx = max / radix, d; + const char *p; + + for (p = str; isxdigit((unsigned char)*p); p++) { + unsigned char ch = (unsigned char)*p; + if (isdigit(ch)) + d = ch - '0'; + else d = tolower(ch - 'a'); + if (d > radix || val > maxx) goto err; + val *= radix; + if (val > max-d) goto err; + val += d; + } + if (p == str || val > max) goto err; + *endptr = p; + return val; +err: + *endptr = "\xff"; + return 0; +} + /* * Return the default port for a scheme */ diff --git a/libfetch/common.h b/libfetch/common.h index dd5c14c..2c16bf7 100644 --- a/libfetch/common.h +++ b/libfetch/common.h @@ -38,6 +38,8 @@ #define FTP_DEFAULT_PROXY_PORT 21 #define HTTP_DEFAULT_PROXY_PORT 3128 +#include <sys/types.h> +#include <limits.h> #include "openssl-compat.h" #if defined(__GNUC__) && __GNUC__ >= 3 @@ -53,6 +55,14 @@ #define HAVE_SA_LEN #endif +#ifndef IPPORT_MAX +# define IPPORT_MAX 65535 +#endif + +#ifndef OFF_MAX +# define OFF_MAX (((((off_t)1 << (sizeof(off_t) * CHAR_BIT - 2)) - 1) << 1) + 1) +#endif + /* Connection */ typedef struct fetchconn conn_t; @@ -86,6 +96,7 @@ struct fetcherr { void fetch_seterr(struct fetcherr *, int); void fetch_syserr(void); void fetch_info(const char *, ...) LIBFETCH_PRINTFLIKE(1, 2); +uintmax_t fetch_parseuint(const char *p, const char **endptr, int radix, uintmax_t max); int fetch_default_port(const char *); int fetch_default_proxy_port(const char *); int fetch_bind(int, int, const char *); @@ -125,7 +136,6 @@ fetchIO *http_request(struct url *, const char *, fetchIO *ftp_request(struct url *, const char *, const char *, struct url_stat *, struct url *, const char *); - /* * Check whether a particular flag is set */ diff --git a/libfetch/fetch.c b/libfetch/fetch.c index a0d4dbd..45c92aa 100644 --- a/libfetch/fetch.c +++ b/libfetch/fetch.c @@ -473,15 +473,12 @@ find_user: /* port */ if (*p == ':') { - for (q = ++p; *q && (*q != '/'); q++) - if (isdigit((unsigned char)*q)) - u->port = u->port * 10 + (*q - '0'); - else { - /* invalid port */ - url_seterr(URL_BAD_PORT); - goto ouch; - } - p = q; + u->port = fetch_parseuint(p + 1, &p, 10, IPPORT_MAX); + if (*p && *p != '/') { + /* invalid port */ + url_seterr(URL_BAD_PORT); + goto ouch; + } } /* document */ diff --git a/libfetch/ftp.c b/libfetch/ftp.c index 8f9f04f..77790aa 100644 --- a/libfetch/ftp.c +++ b/libfetch/ftp.c @@ -471,8 +471,7 @@ ftp_stat(conn_t *conn, const char *file, struct url_stat *us) } for (ln = conn->buf + 4; *ln && isspace((unsigned char)*ln); ln++) /* nothing */ ; - for (us->size = 0; *ln && isdigit((unsigned char)*ln); ln++) - us->size = us->size * 10 + *ln - '0'; + us->size = fetch_parseuint(ln, (const char **) &ln, 10, OFF_MAX); if (*ln && !isspace((unsigned char)*ln)) { ftp_seterr(FTP_PROTOCOL_ERROR); us->size = -1; @@ -700,7 +699,7 @@ retry_mode: if (pasv) { unsigned char addr[64]; - char *ln, *p; + const char *ln, *p; unsigned int i; int port; @@ -737,10 +736,15 @@ retry_mode: for (p = ln + 3; *p && !isdigit((unsigned char)*p); p++) /* nothing */ ; if (!*p) goto protocol_error; - l = (e == FTP_PASSIVE_MODE ? 6 : 21); - for (i = 0; *p && i < l; i++, p++) - addr[i] = strtol(p, &p, 10); - if (i < l) goto protocol_error; + l = (e == FTP_PASSIVE_MODE ? 6 : 21) - 1; + for (i = 0; *p && i < l; i++, p++) { + while (isspace((unsigned char)*p)) p++; + addr[i] = fetch_parseuint(p, &p, 10, UCHAR_MAX); + if (*p != ',') goto protocol_error; + } + while (isspace((unsigned char)*p)) p++; + addr[i] = fetch_parseuint(p, &p, 10, UCHAR_MAX); + if (*p && *p != ')') goto protocol_error; break; case FTP_EPASSIVE_MODE: for (p = ln + 3; *p && *p != '('; p++) diff --git a/libfetch/http.c b/libfetch/http.c index 59d6292..abc3ae6 100644 --- a/libfetch/http.c +++ b/libfetch/http.c @@ -134,29 +134,19 @@ struct httpio static int http_new_chunk(struct httpio *io) { - char *p; + const char *p; if (fetch_getln(io->conn) == -1) - return (-1); + return -1; - if (io->conn->buflen < 2 || !isxdigit((unsigned char)*io->conn->buf)) - return (-1); + if (io->conn->buflen < 2) + return -1; - for (p = io->conn->buf; *p && !isspace((unsigned char)*p); ++p) { - if (*p == ';') - break; - if (!isxdigit((unsigned char)*p)) - return (-1); - if (isdigit((unsigned char)*p)) { - io->chunksize = io->chunksize * 16 + - *p - '0'; - } else { - io->chunksize = io->chunksize * 16 + - 10 + tolower((unsigned char)*p) - 'a'; - } - } + io->chunksize = fetch_parseuint(io->conn->buf, &p, 16, SIZE_MAX); + if (*p && *p != ';' && !isspace(*p)) + return -1; - return (io->chunksize); + return io->chunksize; } /* @@ -502,22 +492,6 @@ http_parse_mtime(const char *p, time_t *mtime) } /* - * Parse a content-length header - */ -static int -http_parse_length(const char *p, off_t *length) -{ - off_t len; - - for (len = 0; *p && isdigit((unsigned char)*p); ++p) - len = len * 10 + (*p - '0'); - if (*p) - return (-1); - *length = len; - return (0); -} - -/* * Parse a content-range header */ static int @@ -532,17 +506,14 @@ http_parse_range(const char *p, off_t *offset, off_t *length, off_t *size) first = last = -1; ++p; } else { - for (first = 0; *p && isdigit((unsigned char)*p); ++p) - first = first * 10 + *p - '0'; + first = fetch_parseuint(p, &p, 10, OFF_MAX); if (*p != '-') return (-1); - for (last = 0, ++p; *p && isdigit((unsigned char)*p); ++p) - last = last * 10 + *p - '0'; + last = fetch_parseuint(p+1, &p, 10, OFF_MAX); } if (first > last || *p != '/') return (-1); - for (len = 0, ++p; *p && isdigit((unsigned char)*p); ++p) - len = len * 10 + *p - '0'; + len = fetch_parseuint(p+1, &p, 10, OFF_MAX); if (*p || len < last - first + 1) return (-1); if (first == -1) @@ -850,7 +821,7 @@ http_request(struct url *URL, const char *op, struct url_stat *us, int e, i, n; off_t offset, clength, length, size; time_t mtime; - const char *p; + const char *p, *q; fetchIO *f; hdr_t h; char hbuf[URL_HOSTLEN + 7], *host; @@ -1050,13 +1021,16 @@ http_request(struct url *URL, const char *op, struct url_stat *us, keep_alive = (strcasecmp(p, "keep-alive") == 0); break; case hdr_content_length: - http_parse_length(p, &clength); + clength = fetch_parseuint(p, &q, 10, OFF_MAX); + if (*q) goto protocol_error; break; case hdr_content_range: - http_parse_range(p, &offset, &length, &size); + if (http_parse_range(p, &offset, &length, &size) < 0) + goto protocol_error; break; case hdr_last_modified: - http_parse_mtime(p, &mtime); + if (http_parse_mtime(p, &mtime) < 0) + goto protocol_error; break; case hdr_location: if (!HTTP_REDIRECT(conn->err)) |