10000 stringToNode() and deparse_expression_pretty() crash on invalid input, · postgres/postgres@fa8ccf5 · GitHub
[go: up one dir, main page]
Skip to content
10000

Commit fa8ccf5

Browse files
hlinnakahlinnaka
committed
stringToNode() and deparse_expression_pretty() crash on invalid input,
but we have nevertheless exposed them to users via pg_get_expr(). It would be too much maintenance effort to rigorously check the input, so put a hack in place instead to restrict pg_get_expr() so that the argument must come from one of the system catalog columns known to contain valid expressions. Per report from Rushabh Lathia. Backpatch to 7.4 which is the oldest supported version at the moment.
1 parent 873b9b8 commit fa8ccf5

File tree

3 files changed

+105
-3
lines changed

3 files changed

+105
-3
lines changed

‎src/backend/parser/parse_expr.c

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.4 2005/11/18 23:08:43 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.5 2010/06/30 18:11:43 heikki Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515

1616
#include "postgres.h"
1717

18+
#include "catalog/catname.h"
19+
#include "catalog/pg_attrdef.h"
20+
#include "catalog/pg_constraint.h"
1821
#include "catalog/pg_operator.h"
1922
#include "catalog/pg_proc.h"
2023
#include "commands/dbcommands.h"
@@ -31,7 +34,9 @@
3134
#include "parser/parse_oper.h"
3235
#include "parser/parse_relation.h"
3336
#include "parser/parse_type.h"
37+
#include "parser/parsetree.h"
3438
#include "utils/builtins.h"
39+
#include "utils/fmgroids.h"
3540
#include "utils/lsyscache.h"
3641
#include "utils/syscache.h"
3742

@@ -447,6 +452,90 @@ transformExpr(ParseState *pstate, Node *expr)
447452
fn->agg_star,
448453
fn->agg_distinct,
449454
false);
455+
456+
/*
457+
* pg_get_expr() is a system function that exposes the
458+
* expression deparsing functionality in ruleutils.c to users.
459+
* Very handy, but it was later realized that the functions in
460+
* ruleutils.c don't check the input rigorously, assuming it to
461+
* come from system catalogs and to therefore be valid. That
462+
* makes it easy for a user to crash the backend by passing a
463+
* maliciously crafted string representation of an expression
464+
* to pg_get_expr().
465+
*
466+
* There's a lot of code in ruleutils.c, so it's not feasible
467+
* to add water-proof input checking after the fact. Even if
468+
* we did it once, it would need to be taken into account in
469+
* any future patches too.
470+
*
471+
* Instead, we restrict pg_rule_expr() to only allow input from
472+
* system catalogs instead. This is a hack, but it's the most
473+
* robust and easiest to backpatch way of plugging the
474+
* vulnerability.
475+
*
476+
* This is transparent to the typical usage pattern of
477+
* "pg_get_expr(systemcolumn, ...)", but will break
478+
* "pg_get_expr('foo', ...)", even if 'foo' is a valid
479+
* expression fetched earlier from a system catalog. Hopefully
480+
* there's isn't many clients doing that out there.
481+
*/
482+
if (result && IsA(result, FuncExpr) && !superuser())
483+
{
484+
FuncExpr *fe = (FuncExpr *) result;
485+
if (fe->funcid == F_PG_GET_EXPR ||
486+
fe->funcid == F_PG_GET_EXPR_EXT)
487+
{
488+
Expr *arg = lfirst(fe->args);
489+
bool allowed = false;
490+
491+
/*
492+
* Check that the argument came directly from one of the
493+
* allowed system catalog columns.
494+
*/
495+
if (IsA(arg, Var))
496+
{
497+
Var *var = (Var *) arg;
498+
RangeTblEntry *rte;
499+
Index levelsup;
500+
501+
levelsup = var->varlevelsup;
502+
while (levelsup-- > 0)
503+
{
504+
pstate = pstate->parentParseState;
505+
Assert(pstate != NULL);
506+
}
507+
Assert(var->varno > 0 &&
508+
(int) var->varno <= length(pstate->p_rtable));
509+
rte = rt_fetch(var->varno, pstate->p_rtable);
510+
511+
if (rte->relid == get_system_catalog_relid(IndexRelationName))
512+
{
513+
if (var->varattno == Anum_pg_index_indexprs ||
514+
var->varattno == Anum_pg_index_indpred)
515+
allowed = true;
516+
}
517+
else if (rte->relid == get_system_catalog_relid(AttrDefaultRelationName))
518+
{
519+
if (var->varattno == Anum_pg_attrdef_adbin)
520+
allowed = true;
521+
}
522+
else if (rte->relid == get_system_catalog_relid(ConstraintRelationName))
523+
{
524+
if (var->varattno == Anum_pg_constraint_conbin)
525+
allowed = true;
526+
}
527+
else if (rte->relid == get_system_catalog_relid(TypeRelationName))
528+
{
529+
if (var->varattno == Anum_pg_type_typdefaultbin)
530+
allowed = true;
531+
}
532+
}
533+
if (!allowed)
534+
ereport(ERROR,
535+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
536+
errmsg("argument to pg_get_expr() must come from system catalogs")));
537+
}
538+
}
450539
break;
451540
}
452541
case T_SubLink:

‎src/backend/tcop/fastpath.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.69 2003/09/25 06:58:02 petere Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.69.2.1 2010/06/30 18:11:43 heikki Exp $
1212
*
1313
* NOTES
1414
* This cruft is the server side of PQfn.
@@ -27,6 +27,7 @@
2727
#include "mb/pg_wchar.h"
2828
#include "tcop/fastpath.h"
2929
#include "utils/acl.h"
30+
#include "utils/fmgroids.h"
3031
#in 341A clude "utils/lsyscache.h"
3132
#include "utils/syscache.h"
3233
#include "utils/tqual.h"
@@ -339,6 +340,16 @@ HandleFunctionRequest(StringInfo msgBuf)
339340
*/
340341
SetQuerySnapshot();
341342

343+
/*
344+
* Restrict access to pg_get_expr(). This reflects the hack in
345+
* transformExpr() in parse_expr.c, see comments there on FuncCall for an
346+
* explanation.
347+
*/
348+
if ((fid == F_PG_GET_EXPR || fid == F_PG_GET_EXPR_EXT) && !superuser())
349+
ereport(ERROR,
350+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
351+
errmsg("argument to pg_get_expr() must come from system catalogs")));
352+
342353
/*
343354
* Prepare function call info block and insert arguments.
344355
*/

‎src/include/catalog/pg_constraint.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $Id: pg_constraint.h,v 1.9 2003/08/08 21:42:32 momjian Exp $
11+
* $Id: pg_constraint.h,v 1.9.4.1 2010/06/30 18:11:43 heikki Exp $
1212
*
1313
* NOTES
1414
* the genbki.sh script reads this file and generates .bki
@@ -19,6 +19,8 @@
1919
#ifndef PG_CONSTRAINT_H
2020
#define PG_CONSTRAINT_H
2121

22+
#include "nodes/pg_list.h"
23+
2224
/* ----------------
2325
* postgres.h contains the system type definitions and the
2426
* CATALOG(), BOOTSTRAP and DATA() sugar words so this file

0 commit comments

Comments
 (0)
0