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

2 files changed, 87 insertions, 0 deletions

diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index c564eedb948..35b6476e501 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -297,7 +297,51 @@ SELECT * FROM CASE_TBL;
 (4 rows)
 
 --
+-- Nested CASE expressions
+--
+-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
+-- with the isNull argument of the inner CASE's ExecEvalCase() call.  After
+-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
+-- the isNull flag for the case test value incorrectly became true, causing
+-- the third WHEN-clause not to match.  The volatile function calls are needed
+-- to prevent constant-folding in the planner, which would hide the bug.
+CREATE FUNCTION vol(text) returns text as
+  'begin return $1; end' language plpgsql volatile;
+SELECT CASE
+  (CASE vol('bar')
+    WHEN 'foo' THEN 'it was foo!'
+    WHEN vol(null) THEN 'null input'
+    WHEN 'bar' THEN 'it was bar!' END
+  )
+  WHEN 'it was foo!' THEN 'foo recognized'
+  WHEN 'it was bar!' THEN 'bar recognized'
+  ELSE 'unrecognized' END;
+      case      
+----------------
+ bar recognized
+(1 row)
+
+-- In this case, we can't inline the SQL function without confusing things.
+CREATE DOMAIN foodomain AS text;
+CREATE FUNCTION volfoo(text) returns foodomain as
+  'begin return $1::foodomain; end' language plpgsql volatile;
+CREATE FUNCTION inline_eq(foodomain, foodomain) returns boolean as
+  'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
+CREATE OPERATOR = (procedure = inline_eq,
+                   leftarg = foodomain, rightarg = foodomain);
+SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' END;
+    case    
+------------
+ is not foo
+(1 row)
+
+--
 -- Clean up
 --
 DROP TABLE CASE_TBL;
 DROP TABLE CASE2_TBL;
+DROP OPERATOR = (foodomain, foodomain);
+DROP FUNCTION inline_eq(foodomain, foodomain);
+DROP FUNCTION volfoo(text);
+DROP DOMAIN foodomain;
+DROP FUNCTION vol(text);
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 5f41753337d..b2377e46109 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -157,8 +157,51 @@ UPDATE CASE_TBL
 SELECT * FROM CASE_TBL;
 
 --
+-- Nested CASE expressions
+--
+
+-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
+-- with the isNull argument of the inner CASE's ExecEvalCase() call.  After
+-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
+-- the isNull flag for the case test value incorrectly became true, causing
+-- the third WHEN-clause not to match.  The volatile function calls are needed
+-- to prevent constant-folding in the planner, which would hide the bug.
+
+CREATE FUNCTION vol(text) returns text as
+  'begin return $1; end' language plpgsql volatile;
+
+SELECT CASE
+  (CASE vol('bar')
+    WHEN 'foo' THEN 'it was foo!'
+    WHEN vol(null) THEN 'null input'
+    WHEN 'bar' THEN 'it was bar!' END
+  )
+  WHEN 'it was foo!' THEN 'foo recognized'
+  WHEN 'it was bar!' THEN 'bar recognized'
+  ELSE 'unrecognized' END;
+
+-- In this case, we can't inline the SQL function without confusing things.
+CREATE DOMAIN foodomain AS text;
+
+CREATE FUNCTION volfoo(text) returns foodomain as
+  'begin return $1::foodomain; end' language plpgsql volatile;
+
+CREATE FUNCTION inline_eq(foodomain, foodomain) returns boolean as
+  'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
+
+CREATE OPERATOR = (procedure = inline_eq,
+                   leftarg = foodomain, rightarg = foodomain);
+
+SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' END;
+
+--
 -- Clean up
 --
 
 DROP TABLE CASE_TBL;
 DROP TABLE CASE2_TBL;
+DROP OPERATOR = (foodomain, foodomain);
+DROP FUNCTION inline_eq(foodomain, foodomain);
+DROP FUNCTION volfoo(text);
+DROP DOMAIN foodomain;
+DROP FUNCTION vol(text);