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

Commit98b0c62

Browse files
committed
Fix two errors with nested CASE/WHEN constructs.
ExecEvalCase() tried to save a cycle or two by passing&econtext->caseValue_isNull as the isNull argument to its sub-evaluation ofthe CASE value expression. If that subexpression itself contained a CASE,then *isNull was an alias for econtext->caseValue_isNull within therecursive call of ExecEvalCase(), leading to confusion about whether theinner call's caseValue was null or not. In the worst case this could leadto a core dump due to dereferencing a null pointer. Fix by not assigningto the global variable until control comes back from the subexpression.Also, avoid using the passed-in isNull pointer transiently for evaluationof WHEN expressions. (Either one of these changes would have beensufficient to fix the known misbehavior, but it's clear now that each ofthese choices was in itself dangerous coding practice and best avoided.There do not seem to be any similar hazards elsewhere in execQual.c.)Also, it was possible for inlining of a SQL function that implements theequality operator used for a CASE comparison to result in one CASEexpression's CaseTestExpr node being inserted inside another CASEexpression. This would certainly result in wrong answers since theimproperly nested CaseTestExpr would be caused to return the inner CASE'scomparison value not the outer's. If the CASE values were of differentdata types, a crash might result; moreover such situations could be abusedto allow disclosure of portions of server memory. To fix, teachinline_function to check for "bare" CaseTestExpr nodes in the arguments ofa function to be inlined, and avoid inlining if there are any.Heikki Linnakangas, Michael Paquier, Tom LaneReport:https://github.com/greenplum-db/gpdb/pull/327Report: <4DDCEEB8.50602@enterprisedb.com>Security:CVE-2016-5423
1 parent286c8bc commit98b0c62

File tree

4 files changed

+185
-5
lines changed

4 files changed

+185
-5
lines changed

‎src/backend/executor/execQual.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2943,19 +2943,30 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
29432943

29442944
/*
29452945
* If there's a test expression, we have to evaluate it and save the value
2946-
* where the CaseTestExpr placeholders can find it. We must save and
2946+
* where the CaseTestExpr placeholders can find it.We must save and
29472947
* restore prior setting of econtext's caseValue fields, in case this node
2948-
* is itself within a larger CASE.
2948+
* is itself within a larger CASE. Furthermore, don't assign to the
2949+
* econtext fields until after returning from evaluation of the test
2950+
* expression. We used to pass &econtext->caseValue_isNull to the
2951+
* recursive call, but that leads to aliasing that variable within said
2952+
* call, which can (and did) produce bugs when the test expression itself
2953+
* contains a CASE.
2954+
*
2955+
* If there's no test expression, we don't actually need to save and
2956+
* restore these fields; but it's less code to just do so unconditionally.
29492957
*/
29502958
save_datum=econtext->caseValue_datum;
29512959
save_isNull=econtext->caseValue_isNull;
29522960

29532961
if (caseExpr->arg)
29542962
{
2963+
boolarg_isNull;
2964+
29552965
econtext->caseValue_datum=ExecEvalExpr(caseExpr->arg,
29562966
econtext,
2957-
&econtext->caseValue_isNull,
2967+
&arg_isNull,
29582968
NULL);
2969+
econtext->caseValue_isNull=arg_isNull;
29592970
}
29602971

29612972
/*
@@ -2967,18 +2978,19 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
29672978
{
29682979
CaseWhenState*wclause=lfirst(clause);
29692980
Datumclause_value;
2981+
boolclause_isNull;
29702982

29712983
clause_value=ExecEvalExpr(wclause->expr,
29722984
econtext,
2973-
isNull,
2985+
&clause_isNull,
29742986
NULL);
29752987

29762988
/*
29772989
* if we have a true test, then we return the result, since the case
29782990
* statement is satisfied. A NULL result from the test is not
29792991
* considered true.
29802992
*/
2981-
if (DatumGetBool(clause_value)&& !*isNull)
2993+
if (DatumGetBool(clause_value)&& !clause_isNull)
29822994
{
29832995
econtext->caseValue_datum=save_datum;
29842996
econtext->caseValue_isNull=save_isNull;

‎src/backend/optimizer/util/clauses.c

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ static bool contain_mutable_functions_walker(Node *node, void *context);
9797
staticboolcontain_volatile_functions_walker(Node*node,void*context);
9898
staticboolcontain_volatile_functions_not_nextval_walker(Node*node,void*context);
9999
staticboolcontain_nonstrict_functions_walker(Node*node,void*context);
100+
staticboolcontain_context_dependent_node(Node*clause);
101+
staticboolcontain_context_dependent_node_walker(Node*node,int*flags);
100102
staticboolcontain_leaked_vars_walker(Node*node,void*context);
101103
staticRelidsfind_nonnullable_rels_walker(Node*node,booltop_level);
102104
staticList*find_nonnullable_vars_walker(Node*node,booltop_level);
@@ -1322,6 +1324,76 @@ contain_nonstrict_functions_walker(Node *node, void *context)
13221324
context);
13231325
}
13241326

1327+
/*****************************************************************************
1328+
*Check clauses for context-dependent nodes
1329+
*****************************************************************************/
1330+
1331+
/*
1332+
* contain_context_dependent_node
1333+
* Recursively search for context-dependent nodes within a clause.
1334+
*
1335+
* CaseTestExpr nodes must appear directly within the corresponding CaseExpr,
1336+
* not nested within another one, or they'll see the wrong test value. If one
1337+
* appears "bare" in the arguments of a SQL function, then we can't inline the
1338+
* SQL function for fear of creating such a situation.
1339+
*
1340+
* CoerceToDomainValue would have the same issue if domain CHECK expressions
1341+
* could get inlined into larger expressions, but presently that's impossible.
1342+
* Still, it might be allowed in future, or other node types with similar
1343+
* issues might get invented. So give this function a generic name, and set
1344+
* up the recursion state to allow multiple flag bits.
1345+
*/
1346+
staticbool
1347+
contain_context_dependent_node(Node*clause)
1348+
{
1349+
intflags=0;
1350+
1351+
returncontain_context_dependent_node_walker(clause,&flags);
1352+
}
1353+
1354+
#defineCCDN_IN_CASEEXPR0x0001/* CaseTestExpr okay here? */
1355+
1356+
staticbool
1357+
contain_context_dependent_node_walker(Node*node,int*flags)
1358+
{
1359+
if (node==NULL)
1360+
return false;
1361+
if (IsA(node,CaseTestExpr))
1362+
return !(*flags&CCDN_IN_CASEEXPR);
1363+
if (IsA(node,CaseExpr))
1364+
{
1365+
CaseExpr*caseexpr= (CaseExpr*)node;
1366+
1367+
/*
1368+
* If this CASE doesn't have a test expression, then it doesn't create
1369+
* a context in which CaseTestExprs should appear, so just fall
1370+
* through and treat it as a generic expression node.
1371+
*/
1372+
if (caseexpr->arg)
1373+
{
1374+
intsave_flags=*flags;
1375+
boolres;
1376+
1377+
/*
1378+
* Note: in principle, we could distinguish the various sub-parts
1379+
* of a CASE construct and set the flag bit only for some of them,
1380+
* since we are only expecting CaseTestExprs to appear in the
1381+
* "expr" subtree of the CaseWhen nodes. But it doesn't really
1382+
* seem worth any extra code. If there are any bare CaseTestExprs
1383+
* elsewhere in the CASE, something's wrong already.
1384+
*/
1385+
*flags |=CCDN_IN_CASEEXPR;
1386+
res=expression_tree_walker(node,
1387+
contain_context_dependent_node_walker,
1388+
(void*)flags);
1389+
*flags=save_flags;
1390+
returnres;
1391+
}
1392+
}
1393+
returnexpression_tree_walker(node,contain_context_dependent_node_walker,
1394+
(void*)flags);
1395+
}
1396+
13251397
/*****************************************************************************
13261398
* Check clauses for Vars passed to non-leakproof functions
13271399
*****************************************************************************/
@@ -4235,6 +4307,8 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
42354307
* doesn't work in the general case because it discards information such
42364308
* as OUT-parameter declarations.
42374309
*
4310+
* Also, context-dependent expression nodes in the argument list are trouble.
4311+
*
42384312
* Returns a simplified expression if successful, or NULL if cannot
42394313
* simplify the function.
42404314
*/
@@ -4429,6 +4503,13 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
44294503
contain_nonstrict_functions(newexpr))
44304504
gotofail;
44314505

4506+
/*
4507+
* If any parameter expression contains a context-dependent node, we can't
4508+
* inline, for fear of putting such a node into the wrong context.
4509+
*/
4510+
if (contain_context_dependent_node((Node*)args))
4511+
gotofail;
4512+
44324513
/*
44334514
* We may be able to do it; there are still checks on parameter usage to
44344515
* make, but those are most easily done in combination with the actual

‎src/test/regress/expected/case.out

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,52 @@ SELECT * FROM CASE_TBL;
296296
-8 | 10.1
297297
(4 rows)
298298

299+
--
300+
-- Nested CASE expressions
301+
--
302+
-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
303+
-- with the isNull argument of the inner CASE's ExecEvalCase() call. After
304+
-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
305+
-- the isNull flag for the case test value incorrectly became true, causing
306+
-- the third WHEN-clause not to match. The volatile function calls are needed
307+
-- to prevent constant-folding in the planner, which would hide the bug.
308+
CREATE FUNCTION vol(text) returns text as
309+
'begin return $1; end' language plpgsql volatile;
310+
SELECT CASE
311+
(CASE vol('bar')
312+
WHEN 'foo' THEN 'it was foo!'
313+
WHEN vol(null) THEN 'null input'
314+
WHEN 'bar' THEN 'it was bar!' END
315+
)
316+
WHEN 'it was foo!' THEN 'foo recognized'
317+
WHEN 'it was bar!' THEN 'bar recognized'
318+
ELSE 'unrecognized' END;
319+
case
320+
----------------
321+
bar recognized
322+
(1 row)
323+
324+
-- In this case, we can't inline the SQL function without confusing things.
325+
CREATE DOMAIN foodomain AS text;
326+
CREATE FUNCTION volfoo(text) returns foodomain as
327+
'begin return $1::foodomain; end' language plpgsql volatile;
328+
CREATE FUNCTION inline_eq(foodomain, foodomain) returns boolean as
329+
'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
330+
CREATE OPERATOR = (procedure = inline_eq,
331+
leftarg = foodomain, rightarg = foodomain);
332+
SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' END;
333+
case
334+
------------
335+
is not foo
336+
(1 row)
337+
299338
--
300339
-- Clean up
301340
--
302341
DROP TABLE CASE_TBL;
303342
DROP TABLE CASE2_TBL;
343+
DROP OPERATOR = (foodomain, foodomain);
344+
DROP FUNCTION inline_eq(foodomain, foodomain);
345+
DROP FUNCTION volfoo(text);
346+
DROP DOMAIN foodomain;
347+
DROP FUNCTION vol(text);

‎src/test/regress/sql/case.sql

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,52 @@ UPDATE CASE_TBL
156156

157157
SELECT*FROM CASE_TBL;
158158

159+
--
160+
-- Nested CASE expressions
161+
--
162+
163+
-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
164+
-- with the isNull argument of the inner CASE's ExecEvalCase() call. After
165+
-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
166+
-- the isNull flag for the case test value incorrectly became true, causing
167+
-- the third WHEN-clause not to match. The volatile function calls are needed
168+
-- to prevent constant-folding in the planner, which would hide the bug.
169+
170+
CREATEFUNCTIONvol(text) returnstextas
171+
'begin return $1; end' language plpgsql volatile;
172+
173+
SELECT CASE
174+
(CASE vol('bar')
175+
WHEN'foo' THEN'it was foo!'
176+
WHEN vol(null) THEN'null input'
177+
WHEN'bar' THEN'it was bar!' END
178+
)
179+
WHEN'it was foo!' THEN'foo recognized'
180+
WHEN'it was bar!' THEN'bar recognized'
181+
ELSE'unrecognized' END;
182+
183+
-- In this case, we can't inline the SQL function without confusing things.
184+
CREATEDOMAINfoodomainAStext;
185+
186+
CREATEFUNCTIONvolfoo(text) returns foodomainas
187+
'begin return $1::foodomain; end' language plpgsql volatile;
188+
189+
CREATEFUNCTIONinline_eq(foodomain, foodomain) returnsbooleanas
190+
'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
191+
192+
CREATE OPERATOR= (procedure= inline_eq,
193+
leftarg= foodomain, rightarg= foodomain);
194+
195+
SELECT CASE volfoo('bar') WHEN'foo'::foodomain THEN'is foo' ELSE'is not foo' END;
196+
159197
--
160198
-- Clean up
161199
--
162200

163201
DROPTABLE CASE_TBL;
164202
DROPTABLE CASE2_TBL;
203+
DROPOPERATOR= (foodomain, foodomain);
204+
DROPFUNCTION inline_eq(foodomain, foodomain);
205+
DROPFUNCTION volfoo(text);
206+
DROPDOMAIN foodomain;
207+
DROPFUNCTION vol(text);

0 commit comments

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