I noticed that the chr() function uses PG_GETARG_UINT32() to get its
argument, even though the argument is a (signed) int. So you get some
slightly silly behavior like this:
=> select chr(-333);
ERROR: 54000: requested character too large for encoding: -333
The attached patch fixes this by accepting the argument using
PG_GETARG_INT32(), doing some checks, and then casting it to unsigned
for the rest of the code.
The patch also fixes another inappropriate use in an example in the
documentation. These two were the only inappropriate uses I found,
after we had fixed a few recently.From 4b14d698486d6c46c3d91c9bf5c380f8acddb095 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 1 Dec 2021 19:18:08 +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.
---
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 @@ <title>Returning Sets</title>
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);
--
2.34.1
