mirror of
https://github.com/postgres/postgres.git
synced 2025-07-08 11:42:09 +03:00
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 of the CASE value expression. If that subexpression itself contained a CASE, then *isNull was an alias for econtext->caseValue_isNull within the recursive call of ExecEvalCase(), leading to confusion about whether the inner call's caseValue was null or not. In the worst case this could lead to a core dump due to dereferencing a null pointer. Fix by not assigning to the global variable until control comes back from the subexpression. Also, avoid using the passed-in isNull pointer transiently for evaluation of WHEN expressions. (Either one of these changes would have been sufficient to fix the known misbehavior, but it's clear now that each of these 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 the equality operator used for a CASE comparison to result in one CASE expression's CaseTestExpr node being inserted inside another CASE expression. This would certainly result in wrong answers since the improperly nested CaseTestExpr would be caused to return the inner CASE's comparison value not the outer's. If the CASE values were of different data types, a crash might result; moreover such situations could be abused to allow disclosure of portions of server memory. To fix, teach inline_function to check for "bare" CaseTestExpr nodes in the arguments of a function to be inlined, and avoid inlining if there are any. Heikki Linnakangas, Michael Paquier, Tom Lane Report: https://github.com/greenplum-db/gpdb/pull/327 Report: <4DDCEEB8.50602@enterprisedb.com> Security: CVE-2016-5423
This commit is contained in:
@ -106,6 +106,8 @@ static bool contain_volatile_functions_not_nextval_walker(Node *node, void *cont
|
||||
static bool has_parallel_hazard_walker(Node *node,
|
||||
has_parallel_hazard_arg *context);
|
||||
static bool contain_nonstrict_functions_walker(Node *node, void *context);
|
||||
static bool contain_context_dependent_node(Node *clause);
|
||||
static bool contain_context_dependent_node_walker(Node *node, int *flags);
|
||||
static bool contain_leaked_vars_walker(Node *node, void *context);
|
||||
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
|
||||
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
|
||||
@ -1334,6 +1336,76 @@ contain_nonstrict_functions_walker(Node *node, void *context)
|
||||
context);
|
||||
}
|
||||
|
||||
/*****************************************************************************
|
||||
* Check clauses for context-dependent nodes
|
||||
*****************************************************************************/
|
||||
|
||||
/*
|
||||
* contain_context_dependent_node
|
||||
* Recursively search for context-dependent nodes within a clause.
|
||||
*
|
||||
* CaseTestExpr nodes must appear directly within the corresponding CaseExpr,
|
||||
* not nested within another one, or they'll see the wrong test value. If one
|
||||
* appears "bare" in the arguments of a SQL function, then we can't inline the
|
||||
* SQL function for fear of creating such a situation.
|
||||
*
|
||||
* CoerceToDomainValue would have the same issue if domain CHECK expressions
|
||||
* could get inlined into larger expressions, but presently that's impossible.
|
||||
* Still, it might be allowed in future, or other node types with similar
|
||||
* issues might get invented. So give this function a generic name, and set
|
||||
* up the recursion state to allow multiple flag bits.
|
||||
*/
|
||||
static bool
|
||||
contain_context_dependent_node(Node *clause)
|
||||
{
|
||||
int flags = 0;
|
||||
|
||||
return contain_context_dependent_node_walker(clause, &flags);
|
||||
}
|
||||
|
||||
#define CCDN_IN_CASEEXPR 0x0001 /* CaseTestExpr okay here? */
|
||||
|
||||
static bool
|
||||
contain_context_dependent_node_walker(Node *node, int *flags)
|
||||
{
|
||||
if (node == NULL)
|
||||
return false;
|
||||
if (IsA(node, CaseTestExpr))
|
||||
return !(*flags & CCDN_IN_CASEEXPR);
|
||||
if (IsA(node, CaseExpr))
|
||||
{
|
||||
CaseExpr *caseexpr = (CaseExpr *) node;
|
||||
|
||||
/*
|
||||
* If this CASE doesn't have a test expression, then it doesn't create
|
||||
* a context in which CaseTestExprs should appear, so just fall
|
||||
* through and treat it as a generic expression node.
|
||||
*/
|
||||
if (caseexpr->arg)
|
||||
{
|
||||
int save_flags = *flags;
|
||||
bool res;
|
||||
|
||||
/*
|
||||
* Note: in principle, we could distinguish the various sub-parts
|
||||
* of a CASE construct and set the flag bit only for some of them,
|
||||
* since we are only expecting CaseTestExprs to appear in the
|
||||
* "expr" subtree of the CaseWhen nodes. But it doesn't really
|
||||
* seem worth any extra code. If there are any bare CaseTestExprs
|
||||
* elsewhere in the CASE, something's wrong already.
|
||||
*/
|
||||
*flags |= CCDN_IN_CASEEXPR;
|
||||
res = expression_tree_walker(node,
|
||||
contain_context_dependent_node_walker,
|
||||
(void *) flags);
|
||||
*flags = save_flags;
|
||||
return res;
|
||||
}
|
||||
}
|
||||
return expression_tree_walker(node, contain_context_dependent_node_walker,
|
||||
(void *) flags);
|
||||
}
|
||||
|
||||
/*****************************************************************************
|
||||
* Check clauses for Vars passed to non-leakproof functions
|
||||
*****************************************************************************/
|
||||
@ -4178,6 +4250,8 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
|
||||
* doesn't work in the general case because it discards information such
|
||||
* as OUT-parameter declarations.
|
||||
*
|
||||
* Also, context-dependent expression nodes in the argument list are trouble.
|
||||
*
|
||||
* Returns a simplified expression if successful, or NULL if cannot
|
||||
* simplify the function.
|
||||
*/
|
||||
@ -4372,6 +4446,13 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
|
||||
contain_nonstrict_functions(newexpr))
|
||||
goto fail;
|
||||
|
||||
/*
|
||||
* If any parameter expression contains a context-dependent node, we can't
|
||||
* inline, for fear of putting such a node into the wrong context.
|
||||
*/
|
||||
if (contain_context_dependent_node((Node *) args))
|
||||
goto fail;
|
||||
|
||||
/*
|
||||
* We may be able to do it; there are still checks on parameter usage to
|
||||
* make, but those are most easily done in combination with the actual
|
||||
|
Reference in New Issue
Block a user