From e9e63b7022ddd0aaaae7cd439daa234cf9e6a21c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 6 Dec 2021 13:26:50 +0100 Subject: [PATCH] Fix inappropriate uses of PG_GETARG_UINT32() The chr() function used PG_GETARG_UINT32() even though the argument is declared as (signed) integer. As a result, you can pass negative arguments to this function and it internally interprets them as positive. Ultimately ends up being harmless, but it seems wrong, so fix this and rearrange the internal error checking a bit to accommodate this. Another case was in the documentation, where example code used PG_GETARG_UINT32() with an argument declared as signed integer. Reviewed-by: Nathan Bossart Discussion: https://www.postgresql.org/message-id/flat/7e43869b-d412-8f81-30a3-809783edc9a3%40enterprisedb.com --- doc/src/sgml/xfunc.sgml | 2 +- src/backend/utils/adt/oracle_compat.c | 33 ++++++++++++++++----------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 584d389d45..0d55909734 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3193,7 +3193,7 @@ retcomposite(PG_FUNCTION_ARGS) oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); /* total number of tuples to be returned */ - funcctx->max_calls = PG_GETARG_UINT32(0); + funcctx->max_calls = PG_GETARG_INT32(0); /* Build a tuple descriptor for our result type */ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) diff --git a/src/backend/utils/adt/oracle_compat.c b/src/backend/utils/adt/oracle_compat.c index f737aa6fbd..ec99f2b738 100644 --- a/src/backend/utils/adt/oracle_compat.c +++ b/src/backend/utils/adt/oracle_compat.c @@ -999,10 +999,26 @@ ascii(PG_FUNCTION_ARGS) Datum chr (PG_FUNCTION_ARGS) { - uint32 cvalue = PG_GETARG_UINT32(0); + int32 arg = PG_GETARG_INT32(0); + uint32 cvalue; text *result; int encoding = GetDatabaseEncoding(); + /* + * Error out on arguments that make no sense or that we can't validly + * represent in the encoding. + */ + if (arg < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("character number must be positive"))); + else if (arg == 0) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("null character not permitted"))); + + cvalue = arg; + if (encoding == PG_UTF8 && cvalue > 127) { /* for Unicode we treat the argument as a code point */ @@ -1017,7 +1033,7 @@ chr (PG_FUNCTION_ARGS) if (cvalue > 0x0010ffff) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested character too large for encoding: %d", + errmsg("requested character too large for encoding: %u", cvalue))); if (cvalue > 0xffff) @@ -1058,28 +1074,19 @@ chr (PG_FUNCTION_ARGS) if (!pg_utf8_islegal(wch, bytes)) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested character not valid for encoding: %d", + errmsg("requested character not valid for encoding: %u", cvalue))); } else { bool is_mb; - /* - * Error out on arguments that make no sense or that we can't validly - * represent in the encoding. - */ - if (cvalue == 0) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("null character not permitted"))); - is_mb = pg_encoding_max_length(encoding) > 1; if ((is_mb && (cvalue > 127)) || (!is_mb && (cvalue > 255))) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested character too large for encoding: %d", + errmsg("requested character too large for encoding: %u", cvalue))); result = (text *) palloc(VARHDRSZ + 1);