diff --git a/contrib/intarray/_int_bool.c b/contrib/intarray/_int_bool.c index 4b6a31080e4..d745ee3a298 100644 --- a/contrib/intarray/_int_bool.c +++ b/contrib/intarray/_int_bool.c @@ -436,35 +436,66 @@ boolop(PG_FUNCTION_ARGS) PG_RETURN_BOOL(result); } +/* + * Recursively fill the "left" fields of an ITEM array that represents + * a valid postfix tree. + * + * ptr: starting element of array + * pos: in/out argument, the array index this call is responsible to fill + * + * At exit, *pos has been decremented to point before the sub-tree whose + * top is the entry-time value of *pos. + */ static void findoprnd(ITEM *ptr, int32 *pos) { + int32 mypos; + /* since this function recurses, it could be driven to stack overflow. */ check_stack_depth(); + /* get the position this call is supposed to update */ + mypos = *pos; + Assert(mypos >= 0); + + /* in all cases, we should decrement *pos to advance over this item */ + (*pos)--; + #ifdef BS_DEBUG - elog(DEBUG3, (ptr[*pos].type == OPR) ? - "%d %c" : "%d %d", *pos, ptr[*pos].val); + elog(DEBUG3, (ptr[mypos].type == OPR) ? + "%d %c" : "%d %d", mypos, ptr[mypos].val); #endif - if (ptr[*pos].type == VAL) + + if (ptr[mypos].type == VAL) { - ptr[*pos].left = 0; - (*pos)--; + /* base case: a VAL has no operand, so just set its left to zero */ + ptr[mypos].left = 0; } - else if (ptr[*pos].val == (int32) '!') + else if (ptr[mypos].val == (int32) '!') { - ptr[*pos].left = -1; - (*pos)--; + /* unary operator, likewise easy: operand is just before it */ + ptr[mypos].left = -1; + /* recurse to scan operand */ findoprnd(ptr, pos); } else { - ITEM *curitem = &ptr[*pos]; - int32 tmp = *pos; + /* binary operator */ + int32 delta; - (*pos)--; + /* recurse to scan right operand */ findoprnd(ptr, pos); - curitem->left = *pos - tmp; + /* we must fill left with offset to left operand's top */ + /* abs(delta) < QUERYTYPEMAXITEMS, so it can't overflow ... */ + delta = *pos - mypos; + /* ... but it might be too large to fit in the 16-bit left field */ + Assert(delta < 0); + if (unlikely(delta < PG_INT16_MIN)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("query_int expression is too complex"))); + ptr[mypos].left = (int16) delta; + /* recurse to scan left operand */ findoprnd(ptr, pos); } } @@ -514,6 +545,7 @@ bqarr_in(PG_FUNCTION_ARGS) query->size = state.num; ptr = GETQUERY(query); + /* fill the query array from the data makepol constructed */ for (i = state.num - 1; i >= 0; i--) { ptr[i].type = state.str->type; @@ -523,8 +555,12 @@ bqarr_in(PG_FUNCTION_ARGS) state.str = tmp; } + /* now fill the "left" fields */ pos = query->size - 1; findoprnd(ptr, &pos); + /* if successful, findoprnd should have scanned the whole array */ + Assert(pos == -1); + #ifdef BS_DEBUG initStringInfo(&pbuf); for (i = 0; i < query->size; i++) diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out index 1c3618dbca9..ae4bc4ea746 100644 --- a/contrib/intarray/expected/_int.out +++ b/contrib/intarray/expected/_int.out @@ -398,6 +398,9 @@ SELECT '1&(2&(4&(5|!6)))'::query_int; 1 & 2 & 4 & ( 5 | !6 ) (1 row) +SELECT (SELECT '0 | ' || string_agg(i::text, ' & ') + FROM generate_series(1, 17000) AS i)::query_int; +ERROR: query_int expression is too complex CREATE TABLE test__int( a int[] ); NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Apache Cloudberry data distribution key for this table. HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql index 0dad7a28fac..1adac50f2fb 100644 --- a/contrib/intarray/sql/_int.sql +++ b/contrib/intarray/sql/_int.sql @@ -74,6 +74,8 @@ SELECT '1&(2&(4&(5&6)))'::query_int; SELECT '1&2&4&5&6'::query_int; SELECT '1&(2&(4&(5|6)))'::query_int; SELECT '1&(2&(4&(5|!6)))'::query_int; +SELECT (SELECT '0 | ' || string_agg(i::text, ' & ') + FROM generate_series(1, 17000) AS i)::query_int; CREATE TABLE test__int( a int[] ); diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index e988669fe14..f829bbf2a36 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1267,6 +1267,9 @@ SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; f (1 row) +SELECT (SELECT 'a | ' || string_agg('b', ' & ') + FROM generate_series(1, 17000) AS i)::ltxtquery; +ERROR: ltxtquery is too large --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; ?column? diff --git a/contrib/ltree/ltxtquery_io.c b/contrib/ltree/ltxtquery_io.c index d967f92110f..cf774b4211f 100644 --- a/contrib/ltree/ltxtquery_io.c +++ b/contrib/ltree/ltxtquery_io.c @@ -271,31 +271,60 @@ makepol(QPRS_STATE *state) return END; } +/* + * Recursively fill the "left" fields of an ITEM array that represents + * a valid postfix tree. + * + * ptr: starting element of array + * pos: in/out argument, the array index this call is responsible to fill + * + * At exit, *pos has been incremented to point after the sub-tree whose + * top is the entry-time value of *pos. + */ static void findoprnd(ITEM *ptr, int32 *pos) { + int32 mypos; + /* since this function recurses, it could be driven to stack overflow. */ check_stack_depth(); - if (ptr[*pos].type == VAL || ptr[*pos].type == VALTRUE) + /* get the position this call is supposed to update */ + mypos = *pos; + + /* in all cases, we should increment *pos to advance over this item */ + (*pos)++; + + if (ptr[mypos].type == VAL || ptr[mypos].type == VALTRUE) { - ptr[*pos].left = 0; - (*pos)++; + /* base case: a VAL has no operand, so just set its left to zero */ + ptr[mypos].left = 0; } - else if (ptr[*pos].val == (int32) '!') + else if (ptr[mypos].val == (int32) '!') { - ptr[*pos].left = 1; - (*pos)++; + /* unary operator, likewise easy: operand is just after it */ + ptr[mypos].left = 1; + /* recurse to scan operand */ findoprnd(ptr, pos); } else { - ITEM *curitem = &ptr[*pos]; - int32 tmp = *pos; + /* binary operator */ + int32 delta; - (*pos)++; + /* recurse to scan right operand */ findoprnd(ptr, pos); - curitem->left = *pos - tmp; + /* we must fill left with offset to left operand's top */ + /* delta can't overflow, see LTXTQUERY_TOO_BIG ... */ + delta = *pos - mypos; + /* ... but it might be too large to fit in the 16-bit left field */ + Assert(delta > 0); + if (unlikely(delta > PG_INT16_MAX)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("ltxtquery is too large"))); + ptr[mypos].left = (int16) delta; + /* recurse to scan left operand */ findoprnd(ptr, pos); } } @@ -372,6 +401,8 @@ queryin(char *buf) /* set left operand's position for every operator */ pos = 0; findoprnd(ptr, &pos); + /* if successful, findoprnd should have scanned the whole array */ + Assert(pos == state.num); return query; } diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 9d430b6ba6e..b2bce33b0af 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -246,6 +246,9 @@ SELECT 'tree.awdfg'::ltree @ 'tree & aWdfg@'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_qw%*'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; +SELECT (SELECT 'a | ' || string_agg('b', ' & ') + FROM generate_series(1, 17000) AS i)::ltxtquery; + --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c index 5a40d56d05d..3bed330e752 100644 --- a/contrib/spi/refint.c +++ b/contrib/spi/refint.c @@ -166,21 +166,24 @@ check_primary_key(PG_FUNCTION_ARGS) if (plan->nplans <= 0) { SPIPlanPtr pplan; - char sql[8192]; + StringInfoData sql; + + initStringInfo(&sql); /* * Construct query: SELECT 1 FROM _referenced_relation_ WHERE Pkey1 = * $1 [AND Pkey2 = $2 [...]] */ - snprintf(sql, sizeof(sql), "select 1 from %s where ", relname); - for (i = 0; i < nkeys; i++) + appendStringInfo(&sql, "select 1 from %s where ", relname); + for (i = 1; i <= nkeys; i++) { - snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s = $%d %s", - args[i + nkeys + 1], i + 1, (i < nkeys - 1) ? "and " : ""); + appendStringInfo(&sql, "%s = $%d ", args[i + nkeys], i); + if (i < nkeys) + appendStringInfoString(&sql, "and "); } /* Prepare plan for query */ - pplan = SPI_prepare(sql, nkeys, argtypes); + pplan = SPI_prepare(sql.data, nkeys, argtypes); if (pplan == NULL) /* internal error */ elog(ERROR, "check_primary_key: SPI_prepare returned %s", SPI_result_code_string(SPI_result)); @@ -196,6 +199,8 @@ check_primary_key(PG_FUNCTION_ARGS) sizeof(SPIPlanPtr)); *(plan->splan) = pplan; plan->nplans = 1; + + pfree(sql.data); } /* @@ -416,7 +421,6 @@ check_foreign_key(PG_FUNCTION_ARGS) if (plan->nplans <= 0) { SPIPlanPtr pplan; - char sql[8192]; char **args2 = args; plan->splan = (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext, @@ -424,6 +428,10 @@ check_foreign_key(PG_FUNCTION_ARGS) for (r = 0; r < nrefs; r++) { + StringInfoData sql; + + initStringInfo(&sql); + relname = args2[0]; /*--------- @@ -437,8 +445,7 @@ check_foreign_key(PG_FUNCTION_ARGS) *--------- */ if (action == 'r') - - snprintf(sql, sizeof(sql), "select 1 from %s where ", relname); + appendStringInfo(&sql, "select 1 from %s where ", relname); /*--------- * For 'C'ascade action we construct DELETE query @@ -465,43 +472,24 @@ check_foreign_key(PG_FUNCTION_ARGS) char *nv; int k; - snprintf(sql, sizeof(sql), "update %s set ", relname); + appendStringInfo(&sql, "update %s set ", relname); for (k = 1; k <= nkeys; k++) { - int is_char_type = 0; - char *type; - fn = SPI_fnumber(tupdesc, args_temp[k - 1]); Assert(fn > 0); /* already checked above */ nv = SPI_getvalue(newtuple, tupdesc, fn); - type = SPI_gettype(tupdesc, fn); - - if (strcmp(type, "text") == 0 || - strcmp(type, "varchar") == 0 || - strcmp(type, "char") == 0 || - strcmp(type, "bpchar") == 0 || - strcmp(type, "date") == 0 || - strcmp(type, "timestamp") == 0) - is_char_type = 1; -#ifdef DEBUG_QUERY - elog(DEBUG4, "check_foreign_key Debug value %s type %s %d", - nv, type, is_char_type); -#endif - /* - * is_char_type =1 i set ' ' for define a new value - */ - snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), - " %s = %s%s%s %s ", - args2[k], (is_char_type > 0) ? "'" : "", - nv, (is_char_type > 0) ? "'" : "", (k < nkeys) ? ", " : ""); + appendStringInfo(&sql, " %s = %s ", + args2[k], quote_literal_cstr(nv)); + if (k < nkeys) + appendStringInfoString(&sql, ", "); } - strcat(sql, " where "); + appendStringInfoString(&sql, " where "); } else /* DELETE */ - snprintf(sql, sizeof(sql), "delete from %s where ", relname); + appendStringInfo(&sql, "delete from %s where ", relname); } @@ -513,25 +501,26 @@ check_foreign_key(PG_FUNCTION_ARGS) */ else if (action == 's') { - snprintf(sql, sizeof(sql), "update %s set ", relname); + appendStringInfo(&sql, "update %s set ", relname); for (i = 1; i <= nkeys; i++) { - snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), - "%s = null%s", - args2[i], (i < nkeys) ? ", " : ""); + appendStringInfo(&sql, "%s = null", args2[i]); + if (i < nkeys) + appendStringInfoString(&sql, ", "); } - strcat(sql, " where "); + appendStringInfoString(&sql, " where "); } /* Construct WHERE qual */ for (i = 1; i <= nkeys; i++) { - snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), "%s = $%d %s", - args2[i], i, (i < nkeys) ? "and " : ""); + appendStringInfo(&sql, "%s = $%d ", args2[i], i); + if (i < nkeys) + appendStringInfoString(&sql, "and "); } /* Prepare plan for query */ - pplan = SPI_prepare(sql, nkeys, argtypes); + pplan = SPI_prepare(sql.data, nkeys, argtypes); if (pplan == NULL) /* internal error */ elog(ERROR, "check_foreign_key: SPI_prepare returned %s", SPI_result_code_string(SPI_result)); @@ -547,11 +536,14 @@ check_foreign_key(PG_FUNCTION_ARGS) plan->splan[r] = pplan; args2 += nkeys + 1; /* to the next relation */ + +#ifdef DEBUG_QUERY + elog(DEBUG4, "check_foreign_key Debug Query is : %s ", sql.data); +#endif + + pfree(sql.data); } plan->nplans = nrefs; -#ifdef DEBUG_QUERY - elog(DEBUG4, "check_foreign_key Debug Query is : %s ", sql); -#endif } /* diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 1d04494cdbb..946c7dd9ebe 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -5731,15 +5731,20 @@ int PQrequestCancel(PGconn *conn); to send simple function calls to the server. - + - This interface is somewhat obsolete, as one can achieve similar + This interface is unsafe and should not be used. When + result_is_int is set to 0, + PQfn may write data beyond the end of + result_buf, regardless of whether the buffer has + enough space for the requested number of bytes. Furthermore, it is + obsolete, as one can achieve similar performance and greater functionality by setting up a prepared statement to define the function call. Then, executing the statement with binary transmission of parameters and results substitutes for a fast-path function call. - + The function PQfnPQfn diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 83849b391ea..fb47f3275ce 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -1570,6 +1570,13 @@ DefineRange(CreateRangeStmt *stmt) /* we can look up the subtype name immediately */ multirangeNamespace = QualifiedNameGetCreationNamespace(defGetQualifiedName(defel), &multirangeTypeName); + + /* Check we have creation rights in target namespace */ + aclresult = pg_namespace_aclcheck(multirangeNamespace, + GetUserId(), ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_SCHEMA, + get_namespace_name(multirangeNamespace)); } else ereport(ERROR, diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index f9e1026a12c..761507efc32 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -542,7 +542,7 @@ scram_verify_plain_password(const char *username, const char *password, * Compare the secret's Server Key with the one computed from the * user-supplied password. */ - return memcmp(computed_key, server_key, SCRAM_KEY_LEN) == 0; + return timingsafe_bcmp(computed_key, server_key, SCRAM_KEY_LEN) == 0; } @@ -1082,9 +1082,9 @@ verify_final_nonce(scram_state *state) if (final_nonce_len != client_nonce_len + server_nonce_len) return false; - if (memcmp(state->client_final_nonce, state->client_nonce, client_nonce_len) != 0) + if (timingsafe_bcmp(state->client_final_nonce, state->client_nonce, client_nonce_len) != 0) return false; - if (memcmp(state->client_final_nonce + client_nonce_len, state->server_nonce, server_nonce_len) != 0) + if (timingsafe_bcmp(state->client_final_nonce + client_nonce_len, state->server_nonce, server_nonce_len) != 0) return false; return true; @@ -1136,7 +1136,7 @@ verify_client_proof(scram_state *state) if (scram_H(ClientKey, SCRAM_KEY_LEN, client_StoredKey) < 0) elog(ERROR, "could not hash stored key"); - if (memcmp(client_StoredKey, state->StoredKey, SCRAM_KEY_LEN) != 0) + if (timingsafe_bcmp(client_StoredKey, state->StoredKey, SCRAM_KEY_LEN) != 0) return false; return true; diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 0809ff4e7ef..b0e1f15c829 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -3873,7 +3873,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por } pfree(cryptvector); - if (memcmp(receivepacket->vector, encryptedpassword, RADIUS_VECTOR_LENGTH) != 0) + if (timingsafe_bcmp(receivepacket->vector, encryptedpassword, RADIUS_VECTOR_LENGTH) != 0) { ereport(LOG, (errmsg("RADIUS response from %s has incorrect MD5 signature", diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index 3fcad991a7e..69a7c67e716 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -196,7 +196,8 @@ md5_crypt_verify(const char *role, const char *shadow_pass, return STATUS_ERROR; } - if (strcmp(client_pass, crypt_pwd) == 0) + if (strlen(client_pass) == strlen(crypt_pwd) && + timingsafe_bcmp(client_pass, crypt_pwd, strlen(crypt_pwd)) == 0) retval = STATUS_OK; else { @@ -261,7 +262,8 @@ plain_crypt_verify(const char *role, const char *shadow_pass, */ return STATUS_ERROR; } - if (strcmp(crypt_client_pass, shadow_pass) == 0) + if (strlen(crypt_client_pass) == strlen(shadow_pass) && + timingsafe_bcmp(crypt_client_pass, shadow_pass, strlen(shadow_pass)) == 0) return STATUS_OK; else { diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 92e830164e8..f0d5d948e58 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2298,6 +2298,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) char *gpqeid = NULL; XLogRecPtr recptr; +retry: pq_startmsgread(); /* @@ -2420,7 +2421,16 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) * another SSL negotiation request, and a GSS request should only * follow if SSL was rejected (client may negotiate in either order) */ - return ProcessStartupPacket(port, true, SSLok == 'S'); + ssl_done = true; + if (SSLok == 'S') + { + /* + * We are done with SSL and negotiated correctly, so consider the + * same for GSS. + */ + gss_done = true; + } + goto retry; } else if (proto == NEGOTIATE_GSS_CODE && !gss_done) { @@ -2464,7 +2474,16 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) * another GSS negotiation request, and an SSL request should only * follow if GSS was rejected (client may negotiate in either order) */ - return ProcessStartupPacket(port, GSSok == 'G', true); + gss_done = true; + if (GSSok == 'G') + { + /* + * We are done with GSS and negotiated correctly, so consider the + * same for SSL. + */ + ssl_done = true; + } + goto retry; } /* Could add additional special packet types here */ diff --git a/src/backend/regex/regc_color.c b/src/backend/regex/regc_color.c index 30bda0e5ad0..7c9dec4bcb0 100644 --- a/src/backend/regex/regc_color.c +++ b/src/backend/regex/regc_color.c @@ -218,6 +218,7 @@ newcolor(struct colormap *cm) n = cm->ncds * 2; if (n > MAX_COLOR + 1) n = MAX_COLOR + 1; + /* the MAX_COLOR+1 limit ensures these alloc sizes can't overflow: */ if (cm->cd == cm->cdspace) { newCd = (struct colordesc *) MALLOC(n * sizeof(struct colordesc)); @@ -434,9 +435,8 @@ newhicolorrow(struct colormap *cm, CERR(REG_ESPACE); return 0; } - newarray = (color *) REALLOC(cm->hicolormap, - cm->maxarrayrows * 2 * - cm->hiarraycols * sizeof(color)); + newarray = REALLOC_ARRAY(cm->hicolormap, color, + cm->maxarrayrows * 2 * cm->hiarraycols); if (newarray == NULL) { CERR(REG_ESPACE); @@ -477,9 +477,8 @@ newhicolorcols(struct colormap *cm) CERR(REG_ESPACE); return; } - newarray = (color *) REALLOC(cm->hicolormap, - cm->maxarrayrows * - cm->hiarraycols * 2 * sizeof(color)); + newarray = REALLOC_ARRAY(cm->hicolormap, color, + cm->maxarrayrows * cm->hiarraycols * 2); if (newarray == NULL) { CERR(REG_ESPACE); @@ -652,8 +651,7 @@ subcoloronechr(struct vars *v, * Potentially, we could need two more colormapranges than we have now, if * the given chr is in the middle of some existing range. */ - newranges = (colormaprange *) - MALLOC((cm->numcmranges + 2) * sizeof(colormaprange)); + newranges = MALLOC_ARRAY(colormaprange, cm->numcmranges + 2); if (newranges == NULL) { CERR(REG_ESPACE); @@ -766,8 +764,7 @@ subcoloronerange(struct vars *v, * Potentially, if we have N non-adjacent ranges, we could need as many as * 2N+1 result ranges (consider case where new range spans 'em all). */ - newranges = (colormaprange *) - MALLOC((cm->numcmranges * 2 + 1) * sizeof(colormaprange)); + newranges = MALLOC_ARRAY(colormaprange, cm->numcmranges * 2 + 1); if (newranges == NULL) { CERR(REG_ESPACE); diff --git a/src/backend/regex/regc_cvec.c b/src/backend/regex/regc_cvec.c index 10306215596..8dbcf3c55e3 100644 --- a/src/backend/regex/regc_cvec.c +++ b/src/backend/regex/regc_cvec.c @@ -40,6 +40,9 @@ /* * newcvec - allocate a new cvec + * + * Note: in current usage, nchrs and nranges are never so large that we risk + * integer overflow in these size calculations, even with 32-bit size_t. */ static struct cvec * newcvec(int nchrs, /* to hold this many chrs... */ diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c index 0e93c74287e..92ae255f6b4 100644 --- a/src/backend/regex/regc_nfa.c +++ b/src/backend/regex/regc_nfa.c @@ -3463,6 +3463,10 @@ compact(struct nfa *nfa, assert(!NISERR()); + /* + * The REG_MAX_COMPILE_SPACE restriction ensures that integer overflow + * can't occur in this loop nor in the allocation requests below. + */ nstates = 0; narcs = 0; for (s = nfa->states; s != NULL; s = s->next) @@ -3515,6 +3519,12 @@ compact(struct nfa *nfa, case LACON: assert(s->no != cnfa->pre); assert(a->co >= 0); + /* make sure the modified color number will fit */ + if (a->co > MAX_COLOR - cnfa->ncolors) + { + NERR(REG_ECOLORS); + return; + } ca->co = (color) (cnfa->ncolors + a->co); ca->to = a->to->no; ca++; diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c index b735fa6eaff..4b0ca021bcf 100644 --- a/src/backend/regex/regcomp.c +++ b/src/backend/regex/regcomp.c @@ -516,6 +516,7 @@ moresubs(struct vars *v, assert(wanted > 0 && (size_t) wanted >= v->nsubs); n = (size_t) wanted * 3 / 2 + 1; + /* n is bounded by the number of states, so no chance of overflow here */ if (v->subs == v->sub10) { p = (struct subre **) MALLOC(n * sizeof(struct subre *)); @@ -2322,8 +2323,8 @@ newlacon(struct vars *v, else { n = v->nlacons; - newlacons = (struct subre *) REALLOC(v->lacons, - (n + 1) * sizeof(struct subre)); + /* better use REALLOC_ARRAY here, as struct subre is big */ + newlacons = REALLOC_ARRAY(v->lacons, struct subre, n + 1); } if (newlacons == NULL) { diff --git a/src/backend/regex/rege_dfa.c b/src/backend/regex/rege_dfa.c index fd90f16e5a2..2c6e3ddb8f9 100644 --- a/src/backend/regex/rege_dfa.c +++ b/src/backend/regex/rege_dfa.c @@ -640,20 +640,29 @@ newdfa(struct vars *v, } else { + /* + * Restrict the ranges of nstates and ncolors enough that the arrays + * we allocate here have no more than INT_MAX members. This protects + * not only the allocation calculations just below, but later indexing + * into these arrays. + */ + if (wordsper >= INT_MAX / (nss + WORK) || + cnfa->ncolors >= INT_MAX / nss) + { + ERR(REG_ETOOBIG); + return NULL; + } d = (struct dfa *) MALLOC(sizeof(struct dfa)); if (d == NULL) { ERR(REG_ESPACE); return NULL; } - d->ssets = (struct sset *) MALLOC(nss * sizeof(struct sset)); - d->statesarea = (unsigned *) MALLOC((nss + WORK) * wordsper * - sizeof(unsigned)); + d->ssets = MALLOC_ARRAY(struct sset, nss); + d->statesarea = MALLOC_ARRAY(unsigned, (nss + WORK) * wordsper); d->work = &d->statesarea[nss * wordsper]; - d->outsarea = (struct sset **) MALLOC(nss * cnfa->ncolors * - sizeof(struct sset *)); - d->incarea = (struct arcp *) MALLOC(nss * cnfa->ncolors * - sizeof(struct arcp)); + d->outsarea = MALLOC_ARRAY(struct sset *, nss * cnfa->ncolors); + d->incarea = MALLOC_ARRAY(struct arcp, nss * cnfa->ncolors); d->ismalloced = true; d->arraysmalloced = true; /* now freedfa() will behave sanely */ diff --git a/src/backend/regex/regexec.c b/src/backend/regex/regexec.c index b743b6a05f4..45c4d9d936c 100644 --- a/src/backend/regex/regexec.c +++ b/src/backend/regex/regexec.c @@ -224,8 +224,7 @@ pg_regexec(regex_t *re, if (v->g->nsub + 1 <= LOCALMAT) v->pmatch = mat; else - v->pmatch = (regmatch_t *) MALLOC((v->g->nsub + 1) * - sizeof(regmatch_t)); + v->pmatch = MALLOC_ARRAY(regmatch_t, v->g->nsub + 1); if (v->pmatch == NULL) return REG_ESPACE; v->nmatch = v->g->nsub + 1; @@ -251,6 +250,7 @@ pg_regexec(regex_t *re, v->subdfas = subdfas; else { + /* ntree is surely less than the number of states, so this is safe: */ v->subdfas = (struct dfa **) MALLOC(n * sizeof(struct dfa *)); if (v->subdfas == NULL) { @@ -265,6 +265,7 @@ pg_regexec(regex_t *re, n = (size_t) v->g->nlacons; if (n > 0) { + /* nlacons is surely less than the number of arcs, so this is safe: */ v->ladfas = (struct dfa **) MALLOC(n * sizeof(struct dfa *)); if (v->ladfas == NULL) { @@ -1143,7 +1144,7 @@ citerdissect(struct vars *v, max_matches = t->max; if (max_matches < min_matches) max_matches = min_matches; - endpts = (chr **) MALLOC((max_matches + 1) * sizeof(chr *)); + endpts = MALLOC_ARRAY(chr *, max_matches + 1); if (endpts == NULL) return REG_ESPACE; endpts[0] = begin; @@ -1350,7 +1351,7 @@ creviterdissect(struct vars *v, max_matches = t->max; if (max_matches < min_matches) max_matches = min_matches; - endpts = (chr **) MALLOC((max_matches + 1) * sizeof(chr *)); + endpts = MALLOC_ARRAY(chr *, max_matches + 1); if (endpts == NULL) return REG_ESPACE; endpts[0] = begin; diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index 26cd7458961..6db81684983 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -5445,6 +5445,7 @@ accumArrayResultArr(ArrayBuildStateArr *astate, ndatabytes; char *data; int i; + int newnitems; /* * We disallow accumulating null subarrays. Another plausible definition @@ -5474,6 +5475,14 @@ accumArrayResultArr(ArrayBuildStateArr *astate, nitems = ArrayGetNItems(ndims, dims); ndatabytes = ARR_SIZE(arg) - ARR_DATA_OFFSET(arg); + /* Check that the array doesn't grow too large */ + newnitems = astate->nitems + nitems; + if (newnitems > MaxArraySize) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("array size exceeds the maximum allowed (%zu)", + MaxArraySize))); + if (astate->ndims == 0) { /* First input; check/save the dimensionality info */ @@ -5539,8 +5548,6 @@ accumArrayResultArr(ArrayBuildStateArr *astate, /* Deal with null bitmap if needed */ if (astate->nullbitmap || ARR_HASNULL(arg)) { - int newnitems = astate->nitems + nitems; - if (astate->nullbitmap == NULL) { /* @@ -5564,7 +5571,7 @@ accumArrayResultArr(ArrayBuildStateArr *astate, nitems); } - astate->nitems += nitems; + astate->nitems = newnitems; astate->dims[0] += 1; MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 7d58e28cd1e..84bb4d47330 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -4037,7 +4037,7 @@ datetime_to_char_body(TmToChar *tmtc, text *fmt, bool is_interval, Oid collid) /* * Allocate workspace for result as C string */ - result = palloc((fmt_len * DCH_MAX_ITEM_SIZ) + 1); + result = palloc(mul_size(fmt_len, DCH_MAX_ITEM_SIZ) + 1); *result = '\0'; if (fmt_len > DCH_CACHE_SIZE) @@ -4048,7 +4048,7 @@ datetime_to_char_body(TmToChar *tmtc, text *fmt, bool is_interval, Oid collid) */ incache = false; - format = (FormatNode *) palloc((fmt_len + 1) * sizeof(FormatNode)); + format = palloc_array(FormatNode, fmt_len + 1); parse_format(format, fmt_str, DCH_keywords, DCH_suff, DCH_index, DCH_FLAG, NULL); @@ -4504,7 +4504,7 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, * Allocate new memory if format picture is bigger than static * cache and do not use cache (call parser always) */ - format = (FormatNode *) palloc((fmt_len + 1) * sizeof(FormatNode)); + format = palloc_array(FormatNode, fmt_len + 1); parse_format(format, fmt_str, DCH_keywords, DCH_suff, DCH_index, DCH_FLAG | (std ? STD_FLAG : 0), NULL); @@ -4979,7 +4979,7 @@ NUM_cache(int len, NUMDesc *Num, text *pars_str, bool *shouldFree) * Allocate new memory if format picture is bigger than static cache * and do not use cache (call parser always) */ - format = (FormatNode *) palloc((len + 1) * sizeof(FormatNode)); + format = palloc_array(FormatNode, len + 1); *shouldFree = true; diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c index 51b0ec95ab4..0cb17a10351 100644 --- a/src/backend/utils/adt/multirangetypes.c +++ b/src/backend/utils/adt/multirangetypes.c @@ -334,7 +334,7 @@ multirange_recv(PG_FUNCTION_ARGS) Oid mltrngtypoid = PG_GETARG_OID(1); int32 typmod = PG_GETARG_INT32(2); MultirangeIOData *cache; - uint32 range_count; + int32 range_count; RangeType **ranges; MultirangeType *ret; StringInfoData tmpbuf; @@ -342,7 +342,8 @@ multirange_recv(PG_FUNCTION_ARGS) cache = get_multirange_io_data(fcinfo, mltrngtypoid, IOFunc_receive); range_count = pq_getmsgint(buf, 4); - ranges = palloc(range_count * sizeof(RangeType *)); + /* palloc_array will enforce a more-or-less-sane range_count value */ + ranges = palloc_array(RangeType *, range_count); initStringInfo(&tmpbuf); for (int i = 0; i < range_count; i++) @@ -828,7 +829,7 @@ multirange_deserialize(TypeCacheEntry *rangetyp, { int i; - *ranges = palloc(*range_count * sizeof(RangeType *)); + *ranges = palloc_array(RangeType *, *range_count); for (i = 0; i < *range_count; i++) (*ranges)[i] = multirange_get_range(rangetyp, multirange, i); } @@ -992,7 +993,7 @@ multirange_constructor2(PG_FUNCTION_ARGS) deconstruct_array(rangeArray, rngtypid, rangetyp->typlen, rangetyp->typbyval, rangetyp->typalign, &elements, &nulls, &range_count); - ranges = palloc0(range_count * sizeof(RangeType *)); + ranges = palloc_array(RangeType *, range_count); for (i = 0; i < range_count; i++) { if (nulls[i]) diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index a9acb875eee..5a6a60c3306 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1804,7 +1804,7 @@ icu_to_uchar(UChar **buff_uchar, const char *buff, size_t nbytes) ereport(ERROR, (errmsg("%s failed: %s", "ucnv_toUChars", u_errorName(status)))); - *buff_uchar = palloc((len_uchar + 1) * sizeof(**buff_uchar)); + *buff_uchar = palloc_array(UChar, len_uchar + 1); status = U_ZERO_ERROR; len_uchar = ucnv_toUChars(icu_converter, *buff_uchar, len_uchar + 1, diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 85a109b5bf4..853dfdc5443 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -1979,15 +1979,19 @@ Datum timeofday(PG_FUNCTION_ARGS) { struct timeval tp; - char templ[128]; - char buf[128]; pg_time_t tt; + struct pg_tm *tm; + char part1[128]; + char part2[128]; + char buf[128 + 128 + 10]; gettimeofday(&tp, NULL); tt = (pg_time_t) tp.tv_sec; - pg_strftime(templ, sizeof(templ), "%a %b %d %H:%M:%S.%%06d %Y %Z", - pg_localtime(&tt, session_timezone)); - snprintf(buf, sizeof(buf), templ, tp.tv_usec); + tm = pg_localtime(&tt, session_timezone); + + pg_strftime(part1, sizeof(part1), "%a %b %d %H:%M:%S", tm); + pg_strftime(part2, sizeof(part2), "%Y %Z", tm); + snprintf(buf, sizeof(buf), "%s.%06d %s", part1, (int) tp.tv_usec, part2); PG_RETURN_TEXT_P(cstring_to_text(buf)); } diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 929d9761a2e..5d51600cb10 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -6265,18 +6265,18 @@ unicode_normalize_func(PG_FUNCTION_ARGS) text *input = PG_GETARG_TEXT_PP(0); char *formstr = text_to_cstring(PG_GETARG_TEXT_PP(1)); UnicodeNormalizationForm form; - int size; + size_t size; pg_wchar *input_chars; pg_wchar *output_chars; unsigned char *p; text *result; - int i; + size_t i; form = unicode_norm_form_from_string(formstr); /* convert to pg_wchar */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); - input_chars = palloc((size + 1) * sizeof(pg_wchar)); + input_chars = palloc_array(pg_wchar, size + 1); p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { @@ -6331,20 +6331,20 @@ unicode_is_normalized(PG_FUNCTION_ARGS) text *input = PG_GETARG_TEXT_PP(0); char *formstr = text_to_cstring(PG_GETARG_TEXT_PP(1)); UnicodeNormalizationForm form; - int size; + size_t size; pg_wchar *input_chars; pg_wchar *output_chars; unsigned char *p; - int i; + size_t i; UnicodeNormalizationQC quickcheck; - int output_size; + size_t output_size; bool result; form = unicode_norm_form_from_string(formstr); /* convert to pg_wchar */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); - input_chars = palloc((size + 1) * sizeof(pg_wchar)); + input_chars = palloc_array(pg_wchar, size + 1); p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index 40f22f4b958..f23ec8e1b4c 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -50,6 +50,9 @@ open_target_file(const char *path, bool trunc) { int mode; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target file path is unsafe for open: \"%s\"", path); + if (dry_run) return; @@ -202,6 +205,9 @@ remove_target_file(const char *path, bool missing_ok) { char dstpath[MAXPGPATH]; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target file path is unsafe for removal: \"%s\"", path); + if (dry_run) return; @@ -222,6 +228,9 @@ truncate_target_file(const char *path, off_t newsize) char dstpath[MAXPGPATH]; int fd; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target file path is unsafe for truncation: \"%s\"", path); + if (dry_run) return; @@ -244,6 +253,10 @@ create_target_dir(const char *path) { char dstpath[MAXPGPATH]; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target directory path is unsafe for directory creation: \"%s\"", + path); + if (dry_run) return; @@ -258,6 +271,10 @@ remove_target_dir(const char *path) { char dstpath[MAXPGPATH]; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target directory path is unsafe for directory removal: \"%s\"", + path); + if (dry_run) return; @@ -272,6 +289,9 @@ create_target_symlink(const char *path, const char *link) { char dstpath[MAXPGPATH]; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target symlink path is unsafe for creation: \"%s\"", path); + if (dry_run) return; @@ -286,6 +306,9 @@ remove_target_symlink(const char *path) { char dstpath[MAXPGPATH]; + if (!path_is_safe_for_extraction(path)) + pg_fatal("target symlink path is unsafe for removal: \"%s\"", path); + if (dry_run) return; diff --git a/src/common/psprintf.c b/src/common/psprintf.c index 52223c38a45..96caed1dc2d 100644 --- a/src/common/psprintf.c +++ b/src/common/psprintf.c @@ -24,9 +24,6 @@ #include "postgres_fe.h" -/* It's possible we could use a different value for this in frontend code */ -#define MaxAllocSize ((Size) 0x3fffffff) /* 1 gigabyte - 1 */ - #endif diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index f177516c26f..ec66210adb0 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -24,9 +24,6 @@ #include "postgres_fe.h" -/* It's possible we could use a different value for this in frontend code */ -#define MaxAllocSize ((Size) 0x3fffffff) /* 1 gigabyte - 1 */ - #endif #include "lib/stringinfo.h" diff --git a/src/common/unicode_norm.c b/src/common/unicode_norm.c index 06bf921e458..783a37eb3de 100644 --- a/src/common/unicode_norm.c +++ b/src/common/unicode_norm.c @@ -23,6 +23,7 @@ #include "common/unicode_norm_hashfunc.h" #include "common/unicode_normprops_table.h" #include "port/pg_bswap.h" +#include "utils/memutils.h" #else #include "common/unicode_norm_table.h" #endif @@ -420,10 +421,28 @@ unicode_normalize(UnicodeNormalizationForm form, const pg_wchar *input) /* * Calculate how many characters long the decomposed version will be. + * + * Some characters decompose to quite a few code points, so that the + * decomposed version's size could overrun MaxAllocSize, and even 32-bit + * size_t, even though the input string presumably fits in that. In + * frontend we want to just return NULL in that case, so monitor the sum + * and exit early once we'd need more than MaxAllocSize bytes. */ decomp_size = 0; for (p = input; *p; p++) + { decomp_size += get_decomposed_size(*p, compat); + if (unlikely(decomp_size > MaxAllocSize / sizeof(pg_wchar))) + { +#ifndef FRONTEND + /* Exit loop and let palloc() throw error below */ + break; +#else + /* Just return NULL with no explicit error */ + return NULL; +#endif + } + } decomp_chars = (pg_wchar *) ALLOC((decomp_size + 1) * sizeof(pg_wchar)); if (decomp_chars == NULL) diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h index a8b59f5db73..04664a80ab3 100644 --- a/src/include/common/fe_memutils.h +++ b/src/include/common/fe_memutils.h @@ -9,6 +9,18 @@ #ifndef FE_MEMUTILS_H #define FE_MEMUTILS_H +/* + * Assumed maximum size for allocation requests. + * + * We don't enforce this, so the actual maximum is the platform's SIZE_MAX. + * But it's useful to have it defined in frontend builds, so that common + * code can check for oversized requests without having frontend-vs-backend + * differences. Also, some code relies on MaxAllocSize being no more than + * INT_MAX/2, so rather than setting this to SIZE_MAX, make it the same as + * the backend's value. + */ +#define MaxAllocSize ((Size) 0x3fffffff) /* 1 gigabyte - 1 */ + /* * Flags for pg_malloc_extended and palloc_extended, deliberately named * the same as the backend flags. diff --git a/src/include/port.h b/src/include/port.h index c30e558a362..240547035a6 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -54,6 +54,7 @@ extern void make_native_path(char *path); extern void cleanup_path(char *path); extern bool path_contains_parent_reference(const char *path); extern bool path_is_relative_and_below_cwd(const char *path); +extern bool path_is_safe_for_extraction(const char *path); extern bool path_is_prefix_of_path(const char *path1, const char *path2); extern char *make_absolute_path(const char *path); extern const char *get_progname(const char *argv0); diff --git a/src/include/regex/regcustom.h b/src/include/regex/regcustom.h index 100c52d640f..fd6595b15a8 100644 --- a/src/include/regex/regcustom.h +++ b/src/include/regex/regcustom.h @@ -50,8 +50,8 @@ #include #endif +#include "common/int.h" #include "mb/pg_wchar.h" - #include "miscadmin.h" /* needed by rcancelrequested/rstacktoodeep */ @@ -60,8 +60,32 @@ #define MALLOC(n) malloc(n) #define FREE(p) free(VS(p)) #define REALLOC(p,n) realloc(VS(p),n) +#define MALLOC_ARRAY(type, n) \ + ((type *) pg_regex_malloc_array(sizeof(type), n)) +#define REALLOC_ARRAY(p, type, n) \ + ((type *) pg_regex_realloc_array(p, sizeof(type), n)) #define assert(x) Assert(x) +static inline void * +pg_regex_malloc_array(size_t s1, size_t s2) +{ + size_t req; + + if (unlikely(pg_mul_size_overflow(s1, s2, &req))) + return NULL; + return malloc(req); +} + +static inline void * +pg_regex_realloc_array(void *p, size_t s1, size_t s2) +{ + size_t req; + + if (unlikely(pg_mul_size_overflow(s1, s2, &req))) + return NULL; + return realloc(p, req); +} + /* internal character type and related */ typedef pg_wchar chr; /* the type itself */ typedef unsigned uchr; /* unsigned type that will hold a chr */ diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h index 8db631c83bb..2c6aa8a9342 100644 --- a/src/include/regex/regguts.h +++ b/src/include/regex/regguts.h @@ -76,6 +76,14 @@ #ifndef FREE #define FREE(p) free(VS(p)) #endif +#ifndef MALLOC_ARRAY +/* we don't depend on calloc's zeroing behavior, we do need overflow check */ +#define MALLOC_ARRAY(type, n) ((type *) calloc(sizeof(type), n)) +#endif +#ifndef REALLOC_ARRAY +/* XXX this definition does not provide the desired overflow check */ +#define REALLOC_ARRAY(p, type, n) ((type *) REALLOC(p, sizeof(type) * (n))) +#endif /* want size of a char in bits, and max value in bounded quantifiers */ #ifndef _POSIX2_RE_DUP_MAX @@ -439,6 +447,11 @@ struct cnfa * (the compacted NFA and the colormap). * The scaling here is based on an empirical measurement that very large * NFAs tend to have about 4 arcs/state. + * + * Do not raise this so high as to allow more than INT_MAX/8 states or arcs, + * or you risk integer overflows in various space allocation requests. + * (We could be more defensive in those places, but that's so far beyond the + * practical range of NFA sizes that it doesn't seem worth additional code.) */ #ifndef REG_MAX_COMPILE_SPACE #define REG_MAX_COMPILE_SPACE \ diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 345f046f807..7303f757bed 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -635,7 +635,7 @@ read_server_first_message(fe_scram_state *state, char *input) /* Verify immediately that the server used our part of the nonce */ if (strlen(nonce) < strlen(state->client_nonce) || - memcmp(nonce, state->client_nonce, strlen(state->client_nonce)) != 0) + timingsafe_bcmp(nonce, state->client_nonce, strlen(state->client_nonce)) != 0) { appendPQExpBufferStr(&conn->errorMessage, libpq_gettext("invalid SCRAM response (nonce mismatch)\n")); @@ -864,7 +864,8 @@ verify_server_signature(fe_scram_state *state, bool *match) pg_hmac_free(ctx); /* signature processed, so now check after it */ - if (memcmp(expected_ServerSignature, state->ServerSignature, SCRAM_KEY_LEN) != 0) + if (timingsafe_bcmp(expected_ServerSignature, state->ServerSignature, + SCRAM_KEY_LEN) != 0) *match = false; else *match = true; diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index ea3a78420ca..109b098b039 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -2920,6 +2920,20 @@ PQfn(PGconn *conn, int result_is_int, const PQArgBlock *args, int nargs) +{ + return PQnfn(conn, fnid, result_buf, -1, result_len, + result_is_int, args, nargs); +} + +/* + * PQnfn + * Private version of PQfn() with verification that returned data fits in + * result_buf when result_is_int == 0. Setting buf_size to -1 disables + * this verification. + */ +PGresult * +PQnfn(PGconn *conn, int fnid, int *result_buf, int buf_size, int *result_len, + int result_is_int, const PQArgBlock *args, int nargs) { *result_len = 0; @@ -2947,7 +2961,7 @@ PQfn(PGconn *conn, } return pqFunctionCall3(conn, fnid, - result_buf, result_len, + result_buf, buf_size, result_len, result_is_int, args, nargs); } diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c index ffd9926dc4e..9c18b6b119d 100644 --- a/src/interfaces/libpq/fe-lobj.c +++ b/src/interfaces/libpq/fe-lobj.c @@ -275,8 +275,8 @@ lo_read(PGconn *conn, int fd, char *buf, size_t len) argv[1].len = 4; argv[1].u.integer = (int) len; - res = PQfn(conn, conn->lobjfuncs->fn_lo_read, - (void *) buf, &result_len, 0, argv, 2); + res = PQnfn(conn, conn->lobjfuncs->fn_lo_read, + (void *) buf, len, &result_len, 0, argv, 2); if (PQresultStatus(res) == PGRES_COMMAND_OK) { PQclear(res); @@ -418,8 +418,8 @@ lo_lseek64(PGconn *conn, int fd, pg_int64 offset, int whence) argv[2].len = 4; argv[2].u.integer = whence; - res = PQfn(conn, conn->lobjfuncs->fn_lo_lseek64, - (void *) &retval, &result_len, 0, argv, 3); + res = PQnfn(conn, conn->lobjfuncs->fn_lo_lseek64, + (void *) &retval, sizeof(retval), &result_len, 0, argv, 3); if (PQresultStatus(res) == PGRES_COMMAND_OK && result_len == 8) { PQclear(res); @@ -574,8 +574,8 @@ lo_tell64(PGconn *conn, int fd) argv[0].len = 4; argv[0].u.integer = fd; - res = PQfn(conn, conn->lobjfuncs->fn_lo_tell64, - (void *) &retval, &result_len, 0, argv, 1); + res = PQnfn(conn, conn->lobjfuncs->fn_lo_tell64, + (void *) &retval, sizeof(retval), &result_len, 0, argv, 1); if (PQresultStatus(res) == PGRES_COMMAND_OK && result_len == 8) { PQclear(res); diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 5311a40a147..4dd146a70e0 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2156,7 +2156,7 @@ pqEndcopy3(PGconn *conn) */ PGresult * pqFunctionCall3(PGconn *conn, Oid fnid, - int *result_buf, int *actual_result_len, + int *result_buf, int buf_size, int *actual_result_len, int result_is_int, const PQArgBlock *args, int nargs) { @@ -2290,6 +2290,18 @@ pqFunctionCall3(PGconn *conn, Oid fnid, } else { + /* + * If the server returned too much data for the + * buffer, something fishy is going on. Abandon ship. + */ + if (buf_size != -1 && *actual_result_len > buf_size) + { + appendPQExpBufferStr(&conn->errorMessage, + libpq_gettext("server returned too much data\n")); + handleSyncLoss(conn, id, *actual_result_len); + return pqPrepareAsyncResult(conn); + } + if (pqGetnchar((char *) result_buf, *actual_result_len, conn)) diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 0cbd611bd98..0a05756cb1e 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -734,6 +734,9 @@ extern PGresult *pqFunctionCall2(PGconn *conn, Oid fnid, extern void pqCommandQueueAdvance(PGconn *conn); extern int PQsendQueryContinue(PGconn *conn, const char *query); +extern PGresult *PQnfn(PGconn *conn, int fnid, int *result_buf, int buf_size, + int *result_len, int result_is_int, + const PQArgBlock *args, int nargs); /* === in fe-protocol3.c === */ @@ -749,7 +752,8 @@ extern int pqGetline3(PGconn *conn, char *s, int maxlen); extern int pqGetlineAsync3(PGconn *conn, char *buffer, int bufsize); extern int pqEndcopy3(PGconn *conn); extern PGresult *pqFunctionCall3(PGconn *conn, Oid fnid, - int *result_buf, int *actual_result_len, + int *result_buf, int buf_size, + int *actual_result_len, int result_is_int, const PQArgBlock *args, int nargs); diff --git a/src/port/path.c b/src/port/path.c index c39d4688cd9..970087eafa2 100644 --- a/src/port/path.c +++ b/src/port/path.c @@ -428,6 +428,23 @@ path_is_relative_and_below_cwd(const char *path) return true; } +/* + * Detect whether a path is safe for use during archive extraction. + * + * This applies canonicalize_path(), then it checks that the path does + * not contain any parent directory references. + */ +bool +path_is_safe_for_extraction(const char *path) +{ + char buf[MAXPGPATH]; + + strlcpy(buf, path, sizeof(buf)); + canonicalize_path(buf); + + return path_is_relative_and_below_cwd(buf); +} + /* * Detect whether path1 is a prefix of path2 (including equality). * diff --git a/src/test/Makefile b/src/test/Makefile index 150c4e97b73..2dac616912d 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -14,7 +14,7 @@ include $(top_builddir)/src/Makefile.global # GPDB_12_MERGE_FEATURE_NOT_SUPPORTED Since logical replication doesn't work for GPDB, # removed "subscription" test from below list. -SUBDIRS = perl regress isolation modules authentication recovery +SUBDIRS = perl postmaster regress isolation modules authentication recovery SUBDIRS += fsync walrep heap_checksum isolation2 fdw singlenode_regress singlenode_isolation2 diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index e3010870f35..588777d1193 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -318,6 +318,82 @@ sub connstr =pod +=item $node->raw_connect() + +Open a raw TCP or Unix domain socket connection to the server. This is +used by low-level protocol and connection limit tests. + +=cut + +sub raw_connect +{ + my ($self) = @_; + my $pgport = $self->port; + my $pghost = $self->host; + + my $socket; + if ($PostgreSQL::Test::Utils::use_unix_sockets) + { + require IO::Socket::UNIX; + my $path = "$pghost/.s.PGSQL.$pgport"; + + $socket = IO::Socket::UNIX->new( + Type => SOCK_STREAM(), + Peer => $path, + ) or die "Cannot create socket - $IO::Socket::errstr\n"; + } + else + { + $socket = IO::Socket::INET->new( + PeerHost => $pghost, + PeerPort => $pgport, + Proto => 'tcp' + ) or die "Cannot create socket - $IO::Socket::errstr\n"; + } + return $socket; +} + +=pod + +=item $node->raw_connect_works() + +Check if raw_connect() function works on this platform. This should +be called to SKIP any tests that require raw_connect(). + +This tries to connect to the server, to test whether it works or not,, +so the server is up and running. Otherwise this can return 0 even if +there's nothing wrong with raw_connect() itself. + +Notably, raw_connect() does not work on Unix domain sockets on +Strawberry perl 5.26.3.1 on Windows, which we use in Cirrus CI images +as of this writing. It dies with "not implemented on this +architecture". + +=cut + +sub raw_connect_works +{ + my ($self) = @_; + + # If we're using Unix domain sockets, we need a working + # IO::Socket::UNIX implementation. + if ($PostgreSQL::Test::Utils::use_unix_sockets) + { + eval { + my $sock = $self->raw_connect(); + $sock->close(); + }; + if ($@ =~ /not implemented/) + { + diag "IO::Socket::UNIX does not work: $@"; + return 0; + } + } + return 1; +} + +=pod + =item $node->group_access() Does the data dir allow group access? diff --git a/src/test/postmaster/.gitignore b/src/test/postmaster/.gitignore new file mode 100644 index 00000000000..871e943d50e --- /dev/null +++ b/src/test/postmaster/.gitignore @@ -0,0 +1,2 @@ +# Generated by test suite +/tmp_check/ diff --git a/src/test/postmaster/Makefile b/src/test/postmaster/Makefile new file mode 100644 index 00000000000..58d2b235830 --- /dev/null +++ b/src/test/postmaster/Makefile @@ -0,0 +1,23 @@ +#------------------------------------------------------------------------- +# +# Makefile for src/test/postmaster +# +# Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group +# Portions Copyright (c) 1994, Regents of the University of California +# +# src/test/postmaster/Makefile +# +#------------------------------------------------------------------------- + +subdir = src/test/postmaster +top_builddir = ../../.. +include $(top_builddir)/src/Makefile.global + +check: + $(prove_check) + +installcheck: + $(prove_installcheck) + +clean distclean maintainer-clean: + rm -rf tmp_check diff --git a/src/test/postmaster/README b/src/test/postmaster/README new file mode 100644 index 00000000000..7e47bf5cff0 --- /dev/null +++ b/src/test/postmaster/README @@ -0,0 +1,27 @@ +src/test/postmaster/README + +Regression tests for postmaster +=============================== + +This directory contains a test suite for postmaster's handling of +connections, connection limits, and startup/shutdown sequence. + + +Running the tests +================= + +NOTE: You must have given the --enable-tap-tests argument to configure. + +Run + make check +or + make installcheck +You can use "make installcheck" if you previously did "make install". +In that case, the code in the installation tree is tested. With +"make check", a temporary installation tree is built from the current +sources and then tested. + +Either way, this test initializes, starts, and stops a test Postgres +cluster. + +See src/test/perl/README for more info about running these tests. diff --git a/src/test/postmaster/t/004_negotiate.pl b/src/test/postmaster/t/004_negotiate.pl new file mode 100644 index 00000000000..f4fbae41137 --- /dev/null +++ b/src/test/postmaster/t/004_negotiate.pl @@ -0,0 +1,81 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test the negotiation of combined SSL and GSS requests. This test +# relies on both SSL and GSS requests to be rejected first, followed +# by more requests. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use Time::HiRes qw(usleep); + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +$node->append_conf('postgresql.conf', "log_min_messages = debug2"); +$node->append_conf('postgresql.conf', + "log_connections = 'on'"); +$node->start; + +if (!$node->raw_connect_works()) +{ + plan skip_all => "this test requires working raw_connect()"; +} + +my $sock = $node->raw_connect(); + +# SSLRequest: packet length followed by NEGOTIATE_SSL_CODE. +my $ssl_request = pack("Nnn", 8, 1234, 5679); + +# GSSENCRequest: packet length followed by NEGOTIATE_GSS_CODE. +my $gss_request = pack("Nnn", 8, 1234, 5680); + +# Send SSLRequest, reject or bypass. +$sock->send($ssl_request); +my $reply = ""; +$sock->recv($reply, 1); +if ($reply ne 'N') +{ + $sock->close(); + plan skip_all => + "server accepted SSL; test requires SSL to be rejected"; +} + +# Send GSSENCRequest, reject or bypass test. +$sock->send($gss_request); +$reply = ""; +$sock->recv($reply, 1); +if ($reply ne 'N') +{ + $sock->close(); + plan skip_all => + "server accepted GSS; test requires GSS to be rejected"; +} + +my $log_offset = -s $node->logfile; + +# Send a second SSLRequest, now that we know that both SSL and GSS have +# been rejected for this connection. We are done with both requests, so +# extra requests will be rejected and fail with an invalid protocol +# version, and the connection should be closed by the server. +$sock->send($ssl_request); + +# Try to read a response, there should be nothing, and certainly not an +# extra 'N' message indicating a rejection. +$reply = ""; +my $bytes = $sock->recv($reply, 1024); +isnt($reply, 'N', + "server does not re-enter SSL negotiation after SSL+GSS were both tried"); + +$sock->close(); +$node->wait_for_log(qr/FATAL: .* unsupported frontend protocol 1234.5679/, + $log_offset); + +# Check extra connection with a simple query. +my $result = $node->safe_psql('postgres', 'select 1;'); +is($result, '1', 'server able to accept connection'); + +$node->stop; + +done_testing(); diff --git a/src/test/regress/expected/multirangetypes.out b/src/test/regress/expected/multirangetypes.out index 55a69f49870..26fbbafc888 100644 --- a/src/test/regress/expected/multirangetypes.out +++ b/src/test/regress/expected/multirangetypes.out @@ -2984,6 +2984,22 @@ select _textrange1(textrange2('a','z')) @> 'b'::text; drop type textrange1; drop type textrange2; -- +-- CREATE TYPE checks for CREATE on multirange schema +-- +create role regress_mr; +create schema mr_sch; +set role regress_mr; +create type mytype as range (subtype=int4, multirange_type_name=mr_sch.mr_type); +ERROR: permission denied for schema mr_sch +reset role; +grant create on schema mr_sch to regress_mr; +set role regress_mr; +create type mytype as range (subtype=int4, multirange_type_name=mr_sch.mr_type); +reset role; +drop type mytype; +drop schema mr_sch; +drop role regress_mr; +-- -- Test polymorphic type system -- create function anyarray_anymultirange_func(a anyarray, r anymultirange) diff --git a/src/test/regress/sql/multirangetypes.sql b/src/test/regress/sql/multirangetypes.sql index e0f5e0d004a..d12b339fc5e 100644 --- a/src/test/regress/sql/multirangetypes.sql +++ b/src/test/regress/sql/multirangetypes.sql @@ -672,6 +672,22 @@ select _textrange1(textrange2('a','z')) @> 'b'::text; drop type textrange1; drop type textrange2; +-- +-- CREATE TYPE checks for CREATE on multirange schema +-- +create role regress_mr; +create schema mr_sch; +set role regress_mr; +create type mytype as range (subtype=int4, multirange_type_name=mr_sch.mr_type); +reset role; +grant create on schema mr_sch to regress_mr; +set role regress_mr; +create type mytype as range (subtype=int4, multirange_type_name=mr_sch.mr_type); +reset role; +drop type mytype; +drop schema mr_sch; +drop role regress_mr; + -- -- Test polymorphic type system -- diff --git a/src/timezone/strftime.c b/src/timezone/strftime.c index dd6c7db8695..3c545026a5b 100644 --- a/src/timezone/strftime.c +++ b/src/timezone/strftime.c @@ -122,6 +122,13 @@ static char *_yconv(int, int, bool, bool, char *, char const *); * Convert timestamp t to string s, a caller-allocated buffer of size maxsize, * using the given format pattern. * + * Unlike standard strftime(), we guarantee to provide a null-terminated + * result even on failure, so long as maxsize > 0. If we overrun the buffer, + * return an empty string rather than risking mis-encoded multibyte output. + * (Since this module only supports C locale, you might think multibyte + * characters are impossible --- but the time zone name printed by %Z comes + * from outside and could contain such.) + * * See also timestamptz_to_str. */ size_t @@ -135,11 +142,15 @@ pg_strftime(char *s, size_t maxsize, const char *format, const struct pg_tm *t) if (!p) { errno = EOVERFLOW; + if (maxsize > 0) + *s = '\0'; return 0; } if (p == s + maxsize) { errno = ERANGE; + if (maxsize > 0) + *s = '\0'; return 0; } *p = '\0';