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

Commitf223bb7

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.This needs to be back-patched further than 9.0, but I don't have timeto do so today. Committing now so that the fix gets into 9.0beta4.
1 parent5b8bd05 commitf223bb7

File tree

4 files changed

+133
-100
lines changed

4 files changed

+133
-100
lines changed

‎src/backend/parser/parse_expr.c‎

Lines changed: 11 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.256 2010/07/06 19:18:57 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.257 2010/07/29 23:16:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515

1616
#include"postgres.h"
1717

18-
#include"catalog/pg_attrdef.h"
19-
#include"catalog/pg_constraint.h"
20-
#include"catalog/pg_proc.h"
2118
#include"catalog/pg_type.h"
2219
#include"commands/dbcommands.h"
2320
#include"miscadmin.h"
@@ -33,7 +30,6 @@
3330
#include"parser/parse_target.h"
3431
#include"parser/parse_type.h"
3532
#include"utils/builtins.h"
36-
#include"utils/fmgroids.h"
3733
#include"utils/lsyscache.h"
3834
#include"utils/xml.h"
3935

@@ -1214,7 +1210,6 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
12141210
{
12151211
List*targs;
12161212
ListCell*args;
1217-
Node*result;
12181213

12191214
/* Transform the list of arguments ... */
12201215
targs=NIL;
@@ -1225,97 +1220,16 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
12251220
}
12261221

12271222
/* ... and hand off to ParseFuncOrColumn */
1228-
result=ParseFuncOrColumn(pstate,
1229-
fn->funcname,
1230-
targs,
1231-
fn->agg_order,
1232-
fn->agg_star,
1233-
fn->agg_distinct,
1234-
fn->func_variadic,
1235-
fn->over,
1236-
false,
1237-
fn->location);
1238-
1239-
/*
1240-
* pg_get_expr() is a system function that exposes the expression
1241-
* deparsing functionality in ruleutils.c to users. Very handy, but it was
1242-
* later realized that the functions in ruleutils.c don't check the input
1243-
* rigorously, assuming it to come from system catalogs and to therefore
1244-
* be valid. That makes it easy for a user to crash the backend by passing
1245-
* a maliciously crafted string representation of an expression to
1246-
* pg_get_expr().
1247-
*
1248-
* There's a lot of code in ruleutils.c, so it's not feasible to add
1249-
* water-proof input checking after the fact. Even if we did it once, it
1250-
* would need to be taken into account in any future patches too.
1251-
*
1252-
* Instead, we restrict pg_rule_expr() to only allow input from system
1253-
* catalogs instead. This is a hack, but it's the most robust and easiest
1254-
* to backpatch way of plugging the vulnerability.
1255-
*
1256-
* This is transparent to the typical usage pattern of
1257-
* "pg_get_expr(systemcolumn, ...)", but will break "pg_get_expr('foo',
1258-
* ...)", even if 'foo' is a valid expression fetched earlier from a
1259-
* system catalog. Hopefully there's isn't many clients doing that out
1260-
* there.
1261-
*/
1262-
if (result&&IsA(result,FuncExpr)&&!superuser())
1263-
{
1264-
FuncExpr*fe= (FuncExpr*)result;
1265-
1266-
if (fe->funcid==F_PG_GET_EXPR||fe->funcid==F_PG_GET_EXPR_EXT)
1267-
{
1268-
Expr*arg=linitial(fe->args);
1269-
boolallowed= false;
1270-
1271-
/*
1272-
* Check that the argument came directly from one of the allowed
1273-
* system catalog columns
1274-
*/
1275-
if (IsA(arg,Var))
1276-
{
1277-
Var*var= (Var*)arg;
1278-
RangeTblEntry*rte;
1279-
1280-
rte=GetRTEByRangeTablePosn(pstate,
1281-
var->varno,var->varlevelsup);
1282-
1283-
switch (rte->relid)
1284-
{
1285-
caseIndexRelationId:
1286-
if (var->varattno==Anum_pg_index_indexprs||
1287-
var->varattno==Anum_pg_index_indpred)
1288-
allowed= true;
1289-
break;
1290-
1291-
caseAttrDefaultRelationId:
1292-
if (var->varattno==Anum_pg_attrdef_adbin)
1293-
allowed= true;
1294-
break;
1295-
1296-
caseProcedureRelationId:
1297-
if (var->varattno==Anum_pg_proc_proargdefaults)
1298-
allowed= true;
1299-
break;
1300-
1301-
caseConstraintRelationId:
1302-
if (var->varattno==Anum_pg_constraint_conbin)
1303-
allowed= true;
1304-
break;
1305-
1306-
caseTypeRelationId:
1307-
if (var->varattno==Anum_pg_type_typdefaultbin)
1308-
allowed= true;
1309-
break;
1310-
}
1311-
}
1312-
if (!allowed)
1313-
ereport(ERROR,
1314-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1315-
errmsg("argument to pg_get_expr() must come from system catalogs")));
1316-
}
1317-
}
1318-
returnresult;
1223+
returnParseFuncOrColumn(pstate,
1224+
fn->funcname,
1225+
targs,
1226+
fn->agg_order,
1227+
fn->agg_star,
1228+
fn->agg_distinct,
1229+
fn->func_variadic,
1230+
fn->over,
1231+
false,
1232+
fn->location);
13191233
}
13201234

13211235
staticNode*

‎src/backend/parser/parse_func.c‎

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,18 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.224 2010/05/30 18:10:40 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.225 2010/07/29 23:16:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include"postgres.h"
1616

17+
#include"catalog/pg_attrdef.h"
18+
#include"catalog/pg_constraint.h"
1719
#include"catalog/pg_proc.h"
1820
#include"catalog/pg_type.h"
1921
#include"funcapi.h"
22+
#include"miscadmin.h"
2023
#include"nodes/makefuncs.h"
2124
#include"nodes/nodeFuncs.h"
2225
#include"parser/parse_agg.h"
@@ -26,6 +29,7 @@
2629
#include"parser/parse_target.h"
2730
#include"parser/parse_type.h"
2831
#include"utils/builtins.h"
32+
#include"utils/fmgroids.h"
2933
#include"utils/lsyscache.h"
3034
#include"utils/syscache.h"
3135

@@ -494,6 +498,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
494498
retval= (Node*)wfunc;
495499
}
496500

501+
/* Hack to protect pg_get_expr() against misuse */
502+
check_pg_get_expr_args(pstate,funcid,fargs);
503+
497504
returnretval;
498505
}
499506

@@ -1580,3 +1587,107 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
15801587

15811588
returnoid;
15821589
}
1590+
1591+
1592+
/*
1593+
* pg_get_expr() is a system function that exposes the expression
1594+
* deparsing functionality in ruleutils.c to users. Very handy, but it was
1595+
* later realized that the functions in ruleutils.c don't check the input
1596+
* rigorously, assuming it to come from system catalogs and to therefore
1597+
* be valid. That makes it easy for a user to crash the backend by passing
1598+
* a maliciously crafted string representation of an expression to
1599+
* pg_get_expr().
1600+
*
1601+
* There's a lot of code in ruleutils.c, so it's not feasible to add
1602+
* water-proof input checking after the fact. Even if we did it once, it
1603+
* would need to be taken into account in any future patches too.
1604+
*
1605+
* Instead, we restrict pg_rule_expr() to only allow input from system
1606+
* catalogs. This is a hack, but it's the most robust and easiest
1607+
* to backpatch way of plugging the vulnerability.
1608+
*
1609+
* This is transparent to the typical usage pattern of
1610+
* "pg_get_expr(systemcolumn, ...)", but will break "pg_get_expr('foo',
1611+
* ...)", even if 'foo' is a valid expression fetched earlier from a
1612+
* system catalog. Hopefully there aren't many clients doing that out there.
1613+
*/
1614+
void
1615+
check_pg_get_expr_args(ParseState*pstate,Oidfnoid,List*args)
1616+
{
1617+
boolallowed= false;
1618+
Node*arg;
1619+
intnetlevelsup;
1620+
1621+
/* if not being called for pg_get_expr, do nothing */
1622+
if (fnoid!=F_PG_GET_EXPR&&fnoid!=F_PG_GET_EXPR_EXT)
1623+
return;
1624+
1625+
/* superusers are allowed to call it anyway (dubious) */
1626+
if (superuser())
1627+
return;
1628+
1629+
/*
1630+
* The first argument must be a Var referencing one of the allowed
1631+
* system-catalog columns. It could be a join alias Var, though.
1632+
*/
1633+
Assert(list_length(args)>1);
1634+
arg= (Node*)linitial(args);
1635+
netlevelsup=0;
1636+
1637+
restart:
1638+
if (IsA(arg,Var))
1639+
{
1640+
Var*var= (Var*)arg;
1641+
RangeTblEntry*rte;
1642+
1643+
netlevelsup+=var->varlevelsup;
1644+
rte=GetRTEByRangeTablePosn(pstate,var->varno,netlevelsup);
1645+
1646+
if (rte->rtekind==RTE_JOIN)
1647+
{
1648+
/* Expand join alias reference */
1649+
if (var->varattno>0&&
1650+
var->varattno <=list_length(rte->joinaliasvars))
1651+
{
1652+
arg= (Node*)list_nth(rte->joinaliasvars,var->varattno-1);
1653+
gotorestart;
1654+
}
1655+
}
1656+
elseif (rte->rtekind==RTE_RELATION)
1657+
{
1658+
switch (rte->relid)
1659+
{
1660+
caseIndexRelationId:
1661+
if (var->varattno==Anum_pg_index_indexprs||
1662+
var->varattno==Anum_pg_index_indpred)
1663+
allowed= true;
1664+
break;
1665+
1666+
caseAttrDefaultRelationId:
1667+
if (var->varattno==Anum_pg_attrdef_adbin)
1668+
allowed= true;
1669+
break;
1670+
1671+
caseProcedureRelationId:
1672+
if (var->varattno==Anum_pg_proc_proargdefaults)
1673+
allowed= true;
1674+
break;
1675+
1676+
caseConstraintRelationId:
1677+
if (var->varattno==Anum_pg_constraint_conbin)
1678+
allowed= true;
1679+
break;
1680+
1681+
caseTypeRelationId:
1682+
if (var->varattno==Anum_pg_type_typdefaultbin)
1683+
allowed= true;
1684+
break;
1685+
}
1686+
}
1687+
}
1688+
1689+
if (!allowed)
1690+
ereport(ERROR,
1691+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1692+
errmsg("argument to pg_get_expr() must come from system catalogs")));
1693+
}

‎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-
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.113 2010/02/26 02:00:52 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.114 2010/07/29 23:16:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -869,6 +869,9 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
869869

870870
ReleaseSysCache(tup);
871871

872+
/* Hack to protect pg_get_expr() against misuse */
873+
check_pg_get_expr_args(pstate,result->opfuncid,args);
874+
872875
return (Expr*)result;
873876
}
874877

@@ -997,6 +1000,9 @@ make_scalar_array_op(ParseState *pstate, List *opname,
9971000

9981001
ReleaseSysCache(tup);
9991002

1003+
/* Hack to protect pg_get_expr() against misuse */
1004+
check_pg_get_expr_args(pstate,result->opfuncid,args);
1005+
10001006
return (Expr*)result;
10011007
}
10021008

‎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-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.68 2010/01/0216:58:07 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.69 2010/07/29 23:16:33 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -82,4 +82,6 @@ extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes,
8282
externOidLookupAggNameTypeNames(List*aggname,List*argtypes,
8383
boolnoError);
8484

85+
externvoidcheck_pg_get_expr_args(ParseState*pstate,Oidfnoid,List*args);
86+
8587
#endif/* PARSE_FUNC_H */

0 commit comments

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