[Libreoffice-commits] core.git: Branch 'libreoffice-7-1' - connectivity/source

[Lionel Elie Mamane (via logerrit)]

 connectivity/source/parse/sqliterator.cxx |   27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

New commits:
commit 97b952d05320f90fe85b91122431d47f3a87ed5d
Author:     Lionel Elie Mamane <lionel at mamane.lu>
AuthorDate: Sat Mar 27 10:57:50 2021 +0100
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Tue Mar 30 11:31:19 2021 +0200

    tdf#141115: correctly find the ORDER BY clause of a UNION
    
    instead of blindly assuming a SELECT is not a UNION, leading to an
    out-of-bounds array access when it is.
    
    Change-Id: I8f904ae65acba8d8ee23b95299058207af68c0ca
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113189
    (cherry picked from commit f4367cfd6978ae2fa896652175956bdbedd3c4bf)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113128
    Tested-by: Jenkins
    Reviewed-by: Lionel Mamane <lionel at mamane.lu>
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>

diff --git a/connectivity/source/parse/sqliterator.cxx b/connectivity/source/parse/sqliterator.cxx
index a91390eca2b8..4d8634d07eb5 100644
--- a/connectivity/source/parse/sqliterator.cxx
+++ b/connectivity/source/parse/sqliterator.cxx
@@ -1831,12 +1831,29 @@ const OSQLParseNode* OSQLParseTreeIterator::getOrderTree() const
 
     // Analyse parse tree (depending on statement type)
     // and set pointer to ORDER clause:
+
+    assert(SQL_ISRULE(m_pParseTree, select_statement) || SQL_ISRULE(m_pParseTree, union_statement));
+
+    auto pParseTree = m_pParseTree;
+    if(SQL_ISRULE(m_pParseTree, union_statement))
+    {
+        assert(m_pParseTree->count() == 4);
+        pParseTree = pParseTree->getChild(3);
+        // since UNION is left-associative (at least in our grammar),
+        // possibly the left-hand (m_pParseTree->getChild(0)) is a union_statement,
+        // but the right hand cannot.
+        assert(SQL_ISRULE(pParseTree, select_statement));
+    }
+
     OSQLParseNode * pOrderClause = nullptr;
-    OSL_ENSURE(m_pParseTree->count() >= 4,"ParseTreeIterator: error in parse tree!");
-    OSQLParseNode * pTableExp = m_pParseTree->getChild(3);
-    OSL_ENSURE(pTableExp != nullptr,"OSQLParseTreeIterator: error in parse tree!");
-    OSL_ENSURE(SQL_ISRULE(pTableExp,table_exp),"OSQLParseTreeIterator: error in parse tree!");
-    OSL_ENSURE(pTableExp->count() == TABLE_EXPRESSION_CHILD_COUNT,"OSQLParseTreeIterator: error in parse tree!");
+    OSL_ENSURE(pParseTree->count() == 4, "OSQLParseTreeIterator::getOrderTree: expected a SELECT, and a SELECT must have exactly four children");
+    OSQLParseNode * pTableExp = pParseTree->getChild(3);
+    OSL_ENSURE(pTableExp != nullptr, "OSQLParseTreeIterator::getOrderTree: got NULL table_exp");
+    OSL_ENSURE(SQL_ISRULE(pTableExp, table_exp), "OSQLParseTreeIterator::getOrderTree: expected table_exp but got something else");
+    OSL_ENSURE(pTableExp->count() == TABLE_EXPRESSION_CHILD_COUNT,"OSQLParseTreeIterator::getOrderTree: table_exp doesn't have the expected number of children");
+    // tdf#141115 upgrade the above to an assert;
+    // this cannot go well if there are too few children
+    assert(pTableExp->count() == TABLE_EXPRESSION_CHILD_COUNT);
 
     pOrderClause = pTableExp->getChild(ORDER_BY_CHILD_POS);
     // If it is an order_by, it must not be empty