On 03/06/10 10:21, Rushabh Lathia wrote:
Server crash while trying to read expression(wrong) using pg_get_expr().
postgres=# SELECT pg_get_expr('{FUNCEXPR', 1255);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
In readfuncs.c, we don't check the return value of pg_strtok, and pass a
NULL to atoi(). The fix is pretty straightforward, we just have to be
more careful with validating the input, see attached patch.
However, I'm afraid we're lacking in input validation of read-funcs in
general. After some random hacking, I found this:
postgres=# SELECT pg_get_expr('{FUNCEXPR 1 2 3 4 4 5 6 7 8 9 9 } }', 1255);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
Which still crashes despite the patch. Does anyone have an idea on how
to validate the input in a more wholesale fashion, so that we don't need
to plug these holes one by one?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index bc6e2a6..0010572 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -57,66 +57,66 @@
/* Read an integer field (anything written as ":fldname %d") */
#define READ_INT_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = atoi(token)
/* Read an unsigned integer field (anything written as ":fldname %u") */
#define READ_UINT_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = atoui(token)
/* Read an OID field (don't hard-wire assumption that OID is same as uint) */
#define READ_OID_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = atooid(token)
/* Read a char field (ie, one ascii character) */
#define READ_CHAR_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = token[0]
/* Read an enumerated-type field that was written as an integer code */
#define READ_ENUM_FIELD(fldname, enumtype) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = (enumtype) atoi(token)
/* Read a float field */
#define READ_FLOAT_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = atof(token)
/* Read a boolean field */
#define READ_BOOL_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = strtobool(token)
/* Read a character-string field */
#define READ_STRING_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = nullable_string(token, length)
/* Read a parse location field (and throw away the value, per notes above) */
#define READ_LOCATION_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
- token = pg_strtok(&length); /* get field value */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* get field value */ \
local_node->fldname = -1 /* set field to "unknown" */
/* Read a Node field */
#define READ_NODE_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
local_node->fldname = nodeRead(NULL, 0)
/* Read a bitmapset field */
#define READ_BITMAPSET_FIELD(fldname) \
- token = pg_strtok(&length); /* skip :fldname */ \
+ token = pg_strtok_mandatory(&length); /* skip :fldname */ \
local_node->fldname = _readBitmapset()
/* Routine exit */
@@ -139,8 +139,20 @@
#define nullable_string(token,length) \
((length) == 0 ? NULL : debackslash(token, length))
-
static Datum readDatum(bool typbyval);
+static char *pg_strtok_mandatory(int *length);
+
+/*
+ * Like pg_strtok(), but throws an error on end of string
+ */
+static char *
+pg_strtok_mandatory(int *length)
+{
+ char *token = pg_strtok(length);
+ if (token == NULL)
+ elog(ERROR, "unexpected end of string while reading tree node");
+ return token;
+}
/*
* _readBitmapset
@@ -427,9 +439,9 @@ _readConst(void)
READ_BOOL_FIELD(constisnull);
READ_LOCATION_FIELD(location);
- token = pg_strtok(&length); /* skip :constvalue */
+ token = pg_strtok_mandatory(&length); /* skip :constvalue */
if (local_node->constisnull)
- token = pg_strtok(&length); /* skip "<>" */
+ token = pg_strtok_mandatory(&length); /* skip "<>" */
else
local_node->constvalue = readDatum(local_node->constbyval);
@@ -640,8 +652,8 @@ _readBoolExpr(void)
READ_LOCALS(BoolExpr);
/* do-it-yourself enum representation */
- token = pg_strtok(&length); /* skip :boolop */
- token = pg_strtok(&length); /* get field value */
+ token = pg_strtok_mandatory(&length); /* skip :boolop */
+ token = pg_strtok_mandatory(&length); /* get field value */
if (strncmp(token, "and", 3) == 0)
local_node->boolop = AND_EXPR;
else if (strncmp(token, "or", 2) == 0)
@@ -1182,7 +1194,7 @@ _readRangeTblEntry(void)
* Given a character string representing a node tree, parseNodeString creates
* the internal node structure.
*
- * The string to be read must already have been loaded into pg_strtok().
+ * The string to be read must already have been loaded into pg_strtok_mandatory().
*/
Node *
parseNodeString(void)
@@ -1191,7 +1203,7 @@ parseNodeString(void)
READ_TEMP_LOCALS();
- token = pg_strtok(&length);
+ token = pg_strtok_mandatory(&length);
#define MATCH(tokname, namelen) \
(length == namelen && strncmp(token, tokname, namelen) == 0)
@@ -1328,7 +1340,7 @@ readDatum(bool typbyval)
/*
* read the actual length of the value
*/
- token = pg_strtok(&tokenLength);
+ token = pg_strtok_mandatory(&tokenLength);
length = atoui(token);
token = pg_strtok(&tokenLength); /* read the '[' */
@@ -1346,7 +1358,7 @@ readDatum(bool typbyval)
s = (char *) (&res);
for (i = 0; i < (Size) sizeof(Datum); i++)
{
- token = pg_strtok(&tokenLength);
+ token = pg_strtok_mandatory(&tokenLength);
s[i] = (char) atoi(token);
}
}
@@ -1357,7 +1369,7 @@ readDatum(bool typbyval)
s = (char *) palloc(length);
for (i = 0; i < length; i++)
{
- token = pg_strtok(&tokenLength);
+ token = pg_strtok_mandatory(&tokenLength);
s[i] = (char) atoi(token);
}
res = PointerGetDatum(s);
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
