Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit10c0565

Browse files
committed
Improved version of patch to protect pg_get_expr() against misuse:
look through join alias Vars to avoid breaking join queries, andmove the test to someplace where it will catch more possible waysof calling a function. We still ought to throw away the whole thingin favor of a data-type-based solution, but that's not feasible inthe back branches.Completion of back-port of my patch of yesterday.
1 parent678e0d9 commit10c0565

File tree

4 files changed

+135
-93
lines changed

4 files changed

+135
-93
lines changed

‎src/backend/parser/parse_expr.c

Lines changed: 1 addition & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.5 2010/06/3018:11:43 heikki Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.6 2010/07/3017:57:32 tgl 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"
2118
#include"catalog/pg_operator.h"
2219
#include"catalog/pg_proc.h"
2320
#include"commands/dbcommands.h"
@@ -34,9 +31,7 @@
3431
#include"parser/parse_oper.h"
3532
#include"parser/parse_relation.h"
3633
#include"parser/parse_type.h"
37-
#include"parser/parsetree.h"
3834
#include"utils/builtins.h"
39-
#include"utils/fmgroids.h"
4035
#include"utils/lsyscache.h"
4136
#include"utils/syscache.h"
4237

@@ -452,90 +447,6 @@ transformExpr(ParseState *pstate, Node *expr)
452447
fn->agg_star,
453448
fn->agg_distinct,
454449
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-
boolallowed= 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-
Indexlevelsup;
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-
elseif (rte->relid==get_system_catalog_relid(AttrDefaultRelationName))
518-
{
519-
if (var->varattno==Anum_pg_attrdef_adbin)
520-
allowed= true;
521-
}
522-
elseif (rte->relid==get_system_catalog_relid(ConstraintRelationName))
523-
{
524-
if (var->varattno==Anum_pg_constraint_conbin)
525-
allowed= true;
526-
}
527-
elseif (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-
}
539450
break;
540451
}
541452
caseT_SubLink:

‎src/backend/parser/parse_func.c

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,22 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.161 2003/09/29 00:05:25 petere Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.161.2.1 2010/07/30 17:57:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include"postgres.h"
1616

1717
#include"access/heapam.h"
1818
#include"catalog/catname.h"
19+
#include"catalog/pg_attrdef.h"
20+
#include"catalog/pg_constraint.h"
1921
#include"catalog/pg_inherits.h"
2022
#include"catalog/pg_proc.h"
2123
#include"lib/stringinfo.h"
24+
#include"miscadmin.h"
2225
#include"nodes/makefuncs.h"
26+
#include"parser/parsetree.h"
2327
#include"parser/parse_agg.h"
2428
#include"parser/parse_coerce.h"
2529
#include"parser/parse_expr.h"
@@ -371,6 +375,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
371375
errmsg("aggregates may not return sets")));
372376
}
373377

378+
/* Hack to protect pg_get_expr() against misuse */
379+
check_pg_get_expr_args(pstate,funcid,fargs);
380+
374381
returnretval;
375382
}
376383

@@ -1531,3 +1538,119 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError)
15311538

15321539
returnLookupFuncName(funcname,argcount,argoids,noError);
15331540
}
1541+
1542+
1543+
/*
1544+
* Given an RT index and nesting depth, find the corresponding RTE.
1545+
* This is the inverse of RTERangeTablePosn.
1546+
*/
1547+
staticRangeTblEntry*
1548+
GetRTEByRangeTablePosn(ParseState*pstate,
1549+
intvarno,
1550+
intsublevels_up)
1551+
{
1552+
while (sublevels_up-->0)
1553+
{
1554+
pstate=pstate->parentParseState;
1555+
Assert(pstate!=NULL);
1556+
}
1557+
Assert(varno>0&&varno <=length(pstate->p_rtable));
1558+
returnrt_fetch(varno,pstate->p_rtable);
1559+
}
1560+
1561+
1562+
/*
1563+
* pg_get_expr() is a system function that exposes the expression
1564+
* deparsing functionality in ruleutils.c to users. Very handy, but it was
1565+
* later realized that the functions in ruleutils.c don't check the input
1566+
* rigorously, assuming it to come from system catalogs and to therefore
1567+
* be valid. That makes it easy for a user to crash the backend by passing
1568+
* a maliciously crafted string representation of an expression to
1569+
* pg_get_expr().
1570+
*
1571+
* There's a lot of code in ruleutils.c, so it's not feasible to add
1572+
* water-proof input checking after the fact. Even if we did it once, it
1573+
* would need to be taken into account in any future patches too.
1574+
*
1575+
* Instead, we restrict pg_rule_expr() to only allow input from system
1576+
* catalogs. This is a hack, but it's the most robust and easiest
1577+
* to backpatch way of plugging the vulnerability.
1578+
*
1579+
* This is transparent to the typical usage pattern of
1580+
* "pg_get_expr(systemcolumn, ...)", but will break "pg_get_expr('foo',
1581+
* ...)", even if 'foo' is a valid expression fetched earlier from a
1582+
* system catalog. Hopefully there aren't many clients doing that out there.
1583+
*/
1584+
void
1585+
check_pg_get_expr_args(ParseState*pstate,Oidfnoid,List*args)
1586+
{
1587+
boolallowed= false;
1588+
Node*arg;
1589+
intnetlevelsup;
1590+
1591+
/* if not being called for pg_get_expr, do nothing */
1592+
if (fnoid!=F_PG_GET_EXPR&&fnoid!=F_PG_GET_EXPR_EXT)
1593+
return;
1594+
1595+
/* superusers are allowed to call it anyway (dubious) */
1596+
if (superuser())
1597+
return;
1598+
1599+
/*
1600+
* The first argument must be a Var referencing one of the allowed
1601+
* system-catalog columns. It could be a join alias Var, though.
1602+
*/
1603+
Assert(args!=NIL);
1604+
arg= (Node*)lfirst(args);
1605+
netlevelsup=0;
1606+
1607+
restart:
1608+
if (IsA(arg,Var))
1609+
{
1610+
Var*var= (Var*)arg;
1611+
RangeTblEntry*rte;
1612+
1613+
netlevelsup+=var->varlevelsup;
1614+
rte=GetRTEByRangeTablePosn(pstate,var->varno,netlevelsup);
1615+
1616+
if (rte->rtekind==RTE_JOIN)
1617+
{
1618+
/* Expand join alias reference */
1619+
if (var->varattno>0&&
1620+
var->varattno <=length(rte->joinaliasvars))
1621+
{
1622+
arg= (Node*)nth(var->varattno-1,rte->joinaliasvars);
1623+
gotorestart;
1624+
}
1625+
}
1626+
elseif (rte->rtekind==RTE_RELATION)
1627+
{
1628+
if (rte->relid==get_system_catalog_relid(IndexRelationName))
1629+
{
1630+
if (var->varattno==Anum_pg_index_indexprs||
1631+
var->varattno==Anum_pg_index_indpred)
1632+
allowed= true;
1633+
}
1634+
elseif (rte->relid==get_system_catalog_relid(AttrDefaultRelationName))
1635+
{
1636+
if (var->varattno==Anum_pg_attrdef_adbin)
1637+
allowed= true;
1638+
}
1639+
elseif (rte->relid==get_system_catalog_relid(ConstraintRelationName))
1640+
{
1641+
if (var->varattno==Anum_pg_constraint_conbin)
1642+
allowed= true;
1643+
}
1644+
elseif (rte->relid==get_system_catalog_relid(TypeRelationName))
1645+
{
1646+
if (var->varattno==Anum_pg_type_typdefaultbin)
1647+
allowed= true;
1648+
}
1649+
}
1650+
}
1651+
1652+
if (!allowed)
1653+
ereport(ERROR,
1654+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1655+
errmsg("argument to pg_get_expr() must come from system catalogs")));
1656+
}

‎src/backend/parser/parse_oper.c

Lines changed: 7 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/parser/parse_oper.c,v 1.76 2003/10/06 20:09:47 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.76.2.1 2010/07/30 17:57:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -932,6 +932,9 @@ make_scalar_array_op(ParseState *pstate, List *opname,
932932
result->useOr=useOr;
933933
result->args=args;
934934

935+
/* Hack to protect pg_get_expr() against misuse */
936+
check_pg_get_expr_args(pstate,opform->oprcode,args);
937+
935938
ReleaseSysCache(tup);
936939

937940
return (Expr*)result;
@@ -1005,5 +1008,8 @@ make_op_expr(ParseState *pstate, Operator op,
10051008
result->opretset=get_func_retset(opform->oprcode);
10061009
result->args=args;
10071010

1011+
/* Hack to protect pg_get_expr() against misuse */
1012+
check_pg_get_expr_args(pstate,opform->oprcode,args);
1013+
10081014
return (Expr*)result;
10091015
}

‎src/include/parser/parse_func.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: parse_func.h,v 1.50 2003/08/04 02:40:14 momjian Exp $
10+
* $Id: parse_func.h,v 1.50.4.1 2010/07/30 17:57:32 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -78,4 +78,6 @@ extern Oid LookupFuncName(List *funcname, int nargs, const Oid *argtypes,
7878
externOidLookupFuncNameTypeNames(List*funcname,List*argtypes,
7979
boolnoError);
8080

81+
externvoidcheck_pg_get_expr_args(ParseState*pstate,Oidfnoid,List*args);
82+
8183
#endif/* PARSE_FUNC_H */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp