backport of #10231

arangodb · KVS85 · Oct 16, 2019 · Oct 16, 2019 · Oct 16, 2019 · Oct 16, 2019
commit bc4154d9fbe59a1690b7c0da9f140a2fbb8f1fd4
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,11 @@
 v3.4.9 (XXX-XX-XX)
 -------------------
 
+* Disallow the usage of subqueries inside AQL traversal PRUNE conditions.
+  Using subqueries inside PRUNE conditions causes undefined behavior,
+  so such queries will now be aborted early on with a parse error 
+  instead of running into undefined behavior.
+
 * Fixed search not working in document view while in code mode.
 
 * Fixed issue #10090: fix repeatable seek to the same document in

diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp
@@ -743,6 +743,23 @@ AstNode* Ast::createNodeReference(Variable const* variable) {
   return node;
 }
 
+/// @brief create an AST subquery reference node
+AstNode* Ast::createNodeSubqueryReference(std::string const& variableName) {
+  AstNode* node = createNode(NODE_TYPE_REFERENCE);
+  node->setFlag(AstNodeFlagType::FLAG_SUBQUERY_REFERENCE);
+
+  auto variable = _scopes.getVariable(variableName);
+
+  if (variable == nullptr) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL,
+                                   "variable not found in reference AstNode");
+  }
+
+  node->setData(variable);
+
+  return node;
+}
+
 /// @brief create an AST variable access
 AstNode* Ast::createNodeAccess(Variable const* variable,
                                std::vector<basics::AttributeName> const& field) {

diff --git a/arangod/Aql/Ast.h b/arangod/Aql/Ast.h
@@ -246,6 +246,9 @@ class Ast {
   /// @brief create an AST reference node
   AstNode* createNodeReference(Variable const* variable);
 
+  /// @brief create an AST subquery reference node
+  AstNode* createNodeSubqueryReference(std::string const& variableName);
+
   /// @brief create an AST parameter node for a value literal
   AstNode* createNodeParameter(char const* name, size_t length);
 

diff --git a/arangod/Aql/AstNode.h b/arangod/Aql/AstNode.h
@@ -71,6 +71,7 @@ enum AstNodeFlagType : AstNodeFlagsType {
   FLAG_KEEP_VARIABLENAME = 0x0010000,  // node is a reference to a variable name, not the variable value (used in KEEP nodes)
   FLAG_BIND_PARAMETER = 0x0020000,     // node was created from a bind parameter
   FLAG_FINALIZED = 0x0040000,  // node has been finalized and should not be modified; only set and checked in maintainer mode
+  FLAG_SUBQUERY_REFERENCE = 0x0080000,  // node references a subquery
 };
 
 /// @brief enumeration of AST node value types

diff --git a/arangod/Aql/grammar.y b/arangod/Aql/grammar.y
@@ -566,7 +566,6 @@ prune_and_options:
       node->addMember($2);
       // Options
       node->addMember($4);
-
     }
   ;
 
@@ -687,6 +686,15 @@ for_statement:
     } prune_and_options {
       auto graphInfoNode = static_cast<AstNode*>(parser->popStack());
       auto variablesNode = static_cast<AstNode*>(parser->popStack());
+
+      auto prune = graphInfoNode->getMember(3);
+      if (prune != nullptr) {
+        Ast::traverseReadOnly(prune, [&](AstNode const* node) {
+          if (node->type == NODE_TYPE_REFERENCE && node->hasFlag(AstNodeFlagType::FLAG_SUBQUERY_REFERENCE)) {
+            parser->registerParseError(TRI_ERROR_QUERY_PARSE, "prune condition must not use a subquery", yylloc.first_line, yylloc.first_column);
+          }
+        });
+      }
       auto node = parser->ast()->createNodeTraversal(variablesNode, graphInfoNode);
       parser->ast()->addOperation(node);
     }
@@ -1416,7 +1424,7 @@ expression_or_query:
       auto subQuery = parser->ast()->createNodeLet(variableName.c_str(), variableName.size(), node, false);
       parser->ast()->addOperation(subQuery);
 
-      $$ = parser->ast()->createNodeReference(variableName);
+      $$ = parser->ast()->createNodeSubqueryReference(variableName);
     }
   ;
 

diff --git a/tests/js/server/aql/aql-graph-traverser.js b/tests/js/server/aql/aql-graph-traverser.js
@@ -2021,6 +2021,20 @@ function complexFilteringSuite() {
 
     tearDownAll: cleanup,
 
+    testPruneWithSubquery: function () {
+      let query = `FOR v,e,p IN 1..100 OUTBOUND @start @ecol PRUNE 2 <= LENGTH(FOR w IN p.vertices FILTER w._id == v._id RETURN 1) RETURN p`;
+      try {
+        let bindVars = {
+          '@eCol': en,
+          'start': vertex.Tri1
+        };
+        db._query(query, bindVars);
+        fail();
+      } catch (err) {
+        assertEqual(err.errorNum, errors.ERROR_QUERY_PARSE.code);
+      }
+    },
+
     testVertexEarlyPruneHighDepth: function () {
       var query = `WITH ${vn}
       FOR v, e, p IN 100 OUTBOUND @start @@eCol