diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 19bbc006669..c3a2448dce5 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -1140,7 +1141,7 @@ parse_comma_separated_list(char **startptr, bool *more) char *p; char *s = *startptr; char *e; - int len; + size_t len; /* * Search for the end of the current element; a comma or end-of-string @@ -5769,7 +5770,21 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options, /* concatenate values into a single string with newline terminators */ size = 1; /* for the trailing null */ for (i = 0; values[i] != NULL; i++) + { + if (values[i]->bv_len >= INT_MAX || + size > (INT_MAX - (values[i]->bv_len + 1))) + { + libpq_append_error(errorMessage, + "connection info string size exceeds the maximum allowed (%d)", + INT_MAX); + ldap_value_free_len(values); + ldap_unbind(ld); + return 3; + } + size += values[i]->bv_len + 1; + } + if ((result = malloc(size)) == NULL) { libpq_append_error(errorMessage, "out of memory"); diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 0b1e37ec30b..dcc8a447d66 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -511,7 +511,7 @@ PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len) } else { - attval->value = (char *) pqResultAlloc(res, len + 1, true); + attval->value = (char *) pqResultAlloc(res, (size_t) len + 1, true); if (!attval->value) goto fail; attval->len = len; @@ -603,8 +603,13 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary) */ if (nBytes >= PGRESULT_SEP_ALLOC_THRESHOLD) { - size_t alloc_size = nBytes + PGRESULT_BLOCK_OVERHEAD; + size_t alloc_size; + /* Don't wrap around with overly large requests. */ + if (nBytes > SIZE_MAX - PGRESULT_BLOCK_OVERHEAD) + return NULL; + + alloc_size = nBytes + PGRESULT_BLOCK_OVERHEAD; block = (PGresult_data *) malloc(alloc_size); if (!block) return NULL; @@ -1274,7 +1279,7 @@ pqRowProcessor(PGconn *conn, const char **errmsgp) bool isbinary = (res->attDescs[i].format != 0); char *val; - val = (char *) pqResultAlloc(res, clen + 1, isbinary); + val = (char *) pqResultAlloc(res, (size_t) clen + 1, isbinary); if (val == NULL) return 0; @@ -4215,6 +4220,27 @@ PQescapeString(char *to, const char *from, size_t length) } +/* + * Frontend version of the backend's add_size(), intended to be API-compatible + * with the pg_add_*_overflow() helpers. Stores the result into *dst on success. + * Returns true instead if the addition overflows. + * + * TODO: move to common/int.h + */ +static bool +add_size_overflow(size_t s1, size_t s2, size_t *dst) +{ + size_t result; + + result = s1 + s2; + if (result < s1 || result < s2) + return true; + + *dst = result; + return false; +} + + /* * Escape arbitrary strings. If as_ident is true, we escape the result * as an identifier; if false, as a literal. The result is returned in @@ -4227,8 +4253,8 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) const char *s; char *result; char *rp; - int num_quotes = 0; /* single or double, depending on as_ident */ - int num_backslashes = 0; + size_t num_quotes = 0; /* single or double, depending on as_ident */ + size_t num_backslashes = 0; size_t input_len = strnlen(str, len); size_t result_size; char quote_char = as_ident ? '"' : '\''; @@ -4294,10 +4320,21 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) } } - /* Allocate output buffer. */ - result_size = input_len + num_quotes + 3; /* two quotes, plus a NUL */ + /* + * Allocate output buffer. Protect against overflow, in case the caller + * has allocated a large fraction of the available size_t. + */ + if (add_size_overflow(input_len, num_quotes, &result_size) || + add_size_overflow(result_size, 3, &result_size)) /* two quotes plus a NUL */ + goto overflow; + if (!as_ident && num_backslashes > 0) - result_size += num_backslashes + 2; + { + if (add_size_overflow(result_size, num_backslashes, &result_size) || + add_size_overflow(result_size, 2, &result_size)) /* for " E" prefix */ + goto overflow; + } + result = rp = (char *) malloc(result_size); if (rp == NULL) { @@ -4370,6 +4407,12 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) *rp = '\0'; return result; + +overflow: + libpq_append_conn_error(conn, + "escaped string size exceeds the maximum allowed (%zu)", + SIZE_MAX); + return NULL; } char * @@ -4435,16 +4478,25 @@ PQescapeByteaInternal(PGconn *conn, unsigned char *result; size_t i; size_t len; - size_t bslash_len = (std_strings ? 1 : 2); + const size_t bslash_len = (std_strings ? 1 : 2); /* - * empty string has 1 char ('\0') + * Calculate the escaped length, watching for overflow as we do with + * PQescapeInternal(). The following code relies on a small constant + * bslash_len so that small additions and multiplications don't need their + * own overflow checks. + * + * Start with the empty string, which has 1 char ('\0'). */ len = 1; if (use_hex) { - len += bslash_len + 1 + 2 * from_length; + /* We prepend "\x" and double each input character. */ + if (add_size_overflow(len, bslash_len + 1, &len) || + add_size_overflow(len, from_length, &len) || + add_size_overflow(len, from_length, &len)) + goto overflow; } else { @@ -4452,13 +4504,25 @@ PQescapeByteaInternal(PGconn *conn, for (i = from_length; i > 0; i--, vp++) { if (*vp < 0x20 || *vp > 0x7e) - len += bslash_len + 3; + { + if (add_size_overflow(len, bslash_len + 3, &len)) /* octal "\ooo" */ + goto overflow; + } else if (*vp == '\'') - len += 2; + { + if (add_size_overflow(len, 2, &len)) /* double each quote */ + goto overflow; + } else if (*vp == '\\') - len += bslash_len + bslash_len; + { + if (add_size_overflow(len, bslash_len * 2, &len)) /* double each backslash */ + goto overflow; + } else - len++; + { + if (add_size_overflow(len, 1, &len)) + goto overflow; + } } } @@ -4519,6 +4583,13 @@ PQescapeByteaInternal(PGconn *conn, *rp = '\0'; return result; + +overflow: + if (conn) + libpq_append_conn_error(conn, + "escaped bytea size exceeds the maximum allowed (%zu)", + SIZE_MAX); + return NULL; } unsigned char * diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c index 6a4de16fe4e..cc667c9bac9 100644 --- a/src/interfaces/libpq/fe-print.c +++ b/src/interfaces/libpq/fe-print.c @@ -104,6 +104,16 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po) } screen_size; #endif + /* + * Quick sanity check on po->fieldSep, since we make heavy use of int + * math throughout. + */ + if (fs_len < strlen(po->fieldSep)) + { + fprintf(stderr, libpq_gettext("overlong field separator\n")); + goto exit; + } + nTups = PQntuples(res); fieldNames = (const char **) calloc(nFields, sizeof(char *)); fieldNotNum = (unsigned char *) calloc(nFields, 1); @@ -391,7 +401,7 @@ do_field(const PQprintOpt *po, const PGresult *res, { if (plen > fieldMax[j]) fieldMax[j] = plen; - if (!(fields[i * nFields + j] = (char *) malloc(plen + 1))) + if (!(fields[i * nFields + j] = (char *) malloc((size_t) plen + 1))) { fprintf(stderr, libpq_gettext("out of memory\n")); return false; @@ -441,6 +451,27 @@ do_field(const PQprintOpt *po, const PGresult *res, } +/* + * Frontend version of the backend's add_size(), intended to be API-compatible + * with the pg_add_*_overflow() helpers. Stores the result into *dst on success. + * Returns true instead if the addition overflows. + * + * TODO: move to common/int.h + */ +static bool +add_size_overflow(size_t s1, size_t s2, size_t *dst) +{ + size_t result; + + result = s1 + s2; + if (result < s1 || result < s2) + return true; + + *dst = result; + return false; +} + + static char * do_header(FILE *fout, const PQprintOpt *po, const int nFields, int *fieldMax, const char **fieldNames, unsigned char *fieldNotNum, @@ -453,15 +484,31 @@ do_header(FILE *fout, const PQprintOpt *po, const int nFields, int *fieldMax, fputs("", fout); else { - int tot = 0; + size_t tot = 0; int n = 0; char *p = NULL; + /* Calculate the border size, checking for overflow. */ for (; n < nFields; n++) - tot += fieldMax[n] + fs_len + (po->standard ? 2 : 0); + { + /* Field plus separator, plus 2 extra '-' in standard format. */ + if (add_size_overflow(tot, fieldMax[n], &tot) || + add_size_overflow(tot, fs_len, &tot) || + (po->standard && add_size_overflow(tot, 2, &tot))) + goto overflow; + } if (po->standard) - tot += fs_len * 2 + 2; - border = malloc(tot + 1); + { + /* An extra separator at the front and back. */ + if (add_size_overflow(tot, fs_len, &tot) || + add_size_overflow(tot, fs_len, &tot) || + add_size_overflow(tot, 2, &tot)) + goto overflow; + } + if (add_size_overflow(tot, 1, &tot)) /* terminator */ + goto overflow; + + border = malloc(tot); if (!border) { fprintf(stderr, libpq_gettext("out of memory\n")); @@ -524,6 +571,10 @@ do_header(FILE *fout, const PQprintOpt *po, const int nFields, int *fieldMax, else fprintf(fout, "\n%s\n", border); return border; + +overflow: + fprintf(stderr, libpq_gettext("header size exceeds the maximum allowed\n")); + return NULL; } diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index da7a8db68c8..838e42e661a 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -16,6 +16,7 @@ #include #include +#include #ifdef WIN32 #include "win32.h" @@ -55,8 +56,8 @@ static int getCopyStart(PGconn *conn, ExecStatusType copytype); static int getReadyForQuery(PGconn *conn); static void reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding); -static int build_startup_packet(const PGconn *conn, char *packet, - const PQEnvironmentOption *options); +static size_t build_startup_packet(const PGconn *conn, char *packet, + const PQEnvironmentOption *options); /* @@ -1234,8 +1235,21 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding) * scridx[] respectively. */ - /* we need a safe allocation size... */ + /* + * We need a safe allocation size. + * + * The only caller of reportErrorPosition() is pqBuildErrorMessage3(); it + * gets its query from either a PQresultErrorField() or a PGcmdQueueEntry, + * both of which must have fit into conn->inBuffer/outBuffer. So slen fits + * inside an int, but we can't assume that (slen * sizeof(int)) fits + * inside a size_t. + */ slen = strlen(wquery) + 1; + if (slen > SIZE_MAX / sizeof(int)) + { + free(wquery); + return; + } qidx = (int *) malloc(slen * sizeof(int)); if (qidx == NULL) @@ -2373,15 +2387,43 @@ pqBuildStartupPacket3(PGconn *conn, int *packetlen, const PQEnvironmentOption *options) { char *startpacket; + size_t len; - *packetlen = build_startup_packet(conn, NULL, options); + len = build_startup_packet(conn, NULL, options); + if (len == 0 || len > INT_MAX) + return NULL; + + *packetlen = len; startpacket = (char *) malloc(*packetlen); if (!startpacket) return NULL; - *packetlen = build_startup_packet(conn, startpacket, options); + + len = build_startup_packet(conn, startpacket, options); + Assert(*packetlen == len); + return startpacket; } +/* + * Frontend version of the backend's add_size(), intended to be API-compatible + * with the pg_add_*_overflow() helpers. Stores the result into *dst on success. + * Returns true instead if the addition overflows. + * + * TODO: move to common/int.h + */ +static bool +add_size_overflow(size_t s1, size_t s2, size_t *dst) +{ + size_t result; + + result = s1 + s2; + if (result < s1 || result < s2) + return true; + + *dst = result; + return false; +} + /* * Build a startup packet given a filled-in PGconn structure. * @@ -2389,13 +2431,13 @@ pqBuildStartupPacket3(PGconn *conn, int *packetlen, * To avoid duplicate logic, this routine is called twice: the first time * (with packet == NULL) just counts the space needed, the second time * (with packet == allocated space) fills it in. Return value is the number - * of bytes used. + * of bytes used, or zero in the unlikely event of size_t overflow. */ -static int +static size_t build_startup_packet(const PGconn *conn, char *packet, const PQEnvironmentOption *options) { - int packet_len = 0; + size_t packet_len = 0; const PQEnvironmentOption *next_eo; const char *val; @@ -2414,10 +2456,12 @@ build_startup_packet(const PGconn *conn, char *packet, do { \ if (packet) \ strcpy(packet + packet_len, optname); \ - packet_len += strlen(optname) + 1; \ + if (add_size_overflow(packet_len, strlen(optname) + 1, &packet_len)) \ + return 0; \ if (packet) \ strcpy(packet + packet_len, optval); \ - packet_len += strlen(optval) + 1; \ + if (add_size_overflow(packet_len, strlen(optval) + 1, &packet_len)) \ + return 0; \ } while(0) if (conn->pguser && conn->pguser[0]) @@ -2452,7 +2496,8 @@ build_startup_packet(const PGconn *conn, char *packet, /* Add trailing terminator */ if (packet) packet[packet_len] = '\0'; - packet_len++; + if (add_size_overflow(packet_len, 1, &packet_len)) + return 0; return packet_len; } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 02c114f1405..0f3661b8889 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -563,7 +563,16 @@ struct pg_conn pg_prng_state prng_state; /* prng state for load balancing connections */ - /* Buffer for data received from backend and not yet processed */ + /* + * Buffer for data received from backend and not yet processed. + * + * NB: We rely on a maximum inBufSize/outBufSize of INT_MAX (and therefore + * an INT_MAX upper bound on the size of any and all packet contents) to + * avoid overflow; for example in reportErrorPosition(). Changing the type + * would require not only an adjustment to the overflow protection in + * pqCheck{In,Out}BufferSpace(), but also a careful audit of all libpq + * code that uses ints during size calculations. + */ char *inBuffer; /* currently allocated buffer */ int inBufSize; /* allocated size of buffer */ int inStart; /* offset to first unconsumed data in buffer */