summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimo Teräs <timo.teras@iki.fi>2021-07-16 10:54:08 +0300
committerTimo Teräs <timo.teras@iki.fi>2021-07-26 14:43:35 +0300
commit36048e8fef019c5be938f8a688845b6eef1d46ab (patch)
tree0aa34f757463289276b3ccacc5395fdc5decca85
parent41a6e4c247e68e906bea1ca7c31f0e8d3b49bc83 (diff)
downloadapk-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.c24
-rw-r--r--libfetch/common.h12
-rw-r--r--libfetch/fetch.c15
-rw-r--r--libfetch/ftp.c18
-rw-r--r--libfetch/http.c62
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))