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

Commit 8f36605

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 221f4de commit 8f36605

File tree

3 files changed

+96
-3
lines changed

3 files changed

+96
-3
lines changed

‎src/backend/parser/parse_expr.c

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.179.4.3 2005/11/18 23:08:28 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.179.4.4 2010/06/30 18:11:31 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"
@@ -32,6 +35,7 @@
3235
#include "parser/parse_relation.h"
3336
#include "parser/parse_type.h"
3437
#include "utils/builtins.h"
38+
#include "utils/fmgroids.h"
3539
#include "utils/lsyscache.h"
3640
#include "utils/syscache.h"
3741

@@ -435,6 +439,82 @@ transformExpr(ParseState *pstate, Node *expr)
435439
fn->agg_star,
436440
fn->agg_distinct,
437441
false);
442+
443+
/*
444+
* pg_get_expr() is a system function that exposes the
445+
* expression deparsing functionality in ruleutils.c to users.
446+
* Very handy, but it was later realized that the functions in
447+
* ruleutils.c don't check the input rigorously, assuming it to
448+
* come from system catalogs and to therefore be valid. That
449+
* makes it easy for a user to crash the backend by passing a
450+
* maliciously crafted string representation of an expression
451+
* to pg_get_expr().
452+
*
453+
* There's a lot of code in ruleutils.c, so it's not feasible
454+
* to add water-proof input checking after the fact. Even if
455+
* we did it once, it would need to be taken into account in
456+
* any future patches too.
457+
*
458+
* Instead, we restrict pg_rule_expr() to only allow input from
459+
* system catalogs instead. This is a hack, but it's the most
460+
* robust and easiest to backpatch way of plugging the
461+
* vulnerability.
462+
*
463+
* This is transparent to the typical usage pattern of
464+
* "pg_get_expr(systemcolumn, ...)", but will break
465+
* "pg_get_expr('foo', ...)", even if 'foo' is a valid
466+
* expression fetched earlier from a system catalog. Hopefully
467+
* there's isn't many clients doing that out there.
468+
*/
469+
if (result && IsA(result, FuncExpr) && !superuser())
470+
{
471+
FuncExpr *fe = (FuncExpr *) result;
472+
if (fe->funcid == F_PG_GET_EXPR ||
473+
fe->funcid == F_PG_GET_EXPR_EXT)
474+
{
475+
Expr *arg = linitial(fe->args);
476+
bool allowed = false;
477+
478+
/*
479+
* Check that the argument came directly from one of the
480+
* allowed system catalog columns
481+
*/
482+
if (IsA(arg, Var))
483+
{
484+
Var *var = (Var *) arg;
485+
RangeTblEntry *rte;
486+
487+
rte = GetRTEByRangeTablePosn(pstate,
488+
var->varno, var->varlevelsup);
489+
490+
if (rte->relid == get_system_catalog_relid(IndexRelationName))
491+
{
492+
if (var->varattno == Anum_pg_index_indexprs ||
493+
var->varattno == Anum_pg_index_indpred)
494+
allowed = true;
495+
}
496+
else if (rte->relid == get_system_catalog_relid(AttrDefaultRelationName))
497+
{
498+
if (var->varattno == Anum_pg_attrdef_adbin)
499+
allowed = true;
500+
}
501+
else if (rte->relid == get_system_catalog_relid(ConstraintRelationName))
502+
{
503+
if (var->varattno == Anum_pg_constraint_conbin)
504+
allowed = true;
505+
}
506+
else if (rte->relid == get_system_catalog_relid(TypeRelationName))
507+
{
508+
if (var->varattno == Anum_pg_type_typdefaultbin)
509+
allowed = true;
510+
}
511+
}
512+
if (!allowed)
513+
ereport(ERROR,
514+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
515+
errmsg("argument to pg_get_expr() must come from system catalogs")));
516+
}
517+
}
438518
break;
439519
}
440520
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-
* $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.77.4.1 2006/06/11 15:49:46 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.77.4.2 2010/06/30 18:11:32 heikki Exp $
1212
*
1313
* NOTES
1414
* This cruft is the server side of PQfn.
@@ -28,6 +28,7 @@
2828
#include "tcop/fastpath.h"
2929
#include "tcop/tcopprot.h"
3030
#include "utils/acl.h"
31+
#include "utils/fmgroids.h"
3132
#include "utils/lsyscache.h"
3233
#include "utils/syscache.h"
3334
#include "utils/tqual.h"
@@ -345,6 +346,16 @@ HandleFunctionRequest(StringInfo msgBuf)
345346
aclcheck_error(aclresult, ACL_KIND_PROC,
346347
get_func_name(fid));
347348

349+
/*
350+
* Restrict access to pg_get_expr(). This reflects the hack in
351+
* transformExpr() in parse_expr.c, see comments there on FuncCall for an
352+
* explanation.
353+
*/
354+
if ((fid == F_PG_GET_EXPR || fid == F_PG_GET_EXPR_EXT) && !superuser())
355+
ereport(ERROR,
356+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
357+
errmsg("argument to pg_get_expr() must come from system catalogs")));
358+
348359
/*
349360
* Prepare function call info block and insert arguments.
350361
*/

‎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-2005, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.14 2004/12/31 22:03:24 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.14.4.1 2010/06/30 18:11:32 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