Fix #15123: clang import: reject AST nodes without the ext tokens the handlers read (#8948)

Follow-up to #8939. Several `createTokens` handlers call
`mExtTokens.back()`/`front()` or index relative to `size()` without
checking that the node line had any ext tokens, so a `--clang` dump line
with nothing after the node name (e.g. `` `-IntegerLiteral `` followed
by a space) reads out of bounds. The same happens for StringLiteral,
CharacterLiteral, CXXBoolLiteralExpr, LabelStmt, EnumDecl, DeclRefExpr
and others, for a VarDecl that only has an address
(`mExtTokens[typeIndex - 1]` with `typeIndex` 0), and for a FunctionDecl
ending in `prev` (dereferences `end()`).

The AstNode constructor now throws an AST internal error when the node
has no ext tokens, and `createTokensVarDecl` /
`createTokensFunctionDecl` throw the same error for the other two
shapes. An address-only FunctionDecl / CXXMethodDecl /
CXXConstructorDecl or EnumConstantDecl (`` `-FunctionDecl 0x1 ``) is
rejected as well: `getSpelling()` is empty there, `addtoken("")` adds
nothing and returns a null `back()`, so both handlers now check the
spelling before adding any token. Real clang dumps always carry the node
address, location, name and type, so they are unaffected (checked
`--clang` on the samples and a few lib files). `crash2` now expects the
internal error instead of silently producing no tokens. The size()-2
sites are guarded in #8939 and not touched here.

Author: Nussu06

lib/clangimport.cpp

@@ -336,7 +336,10 @@ namespace clangimport {
     public:
         AstNode(std::string nodeType, const std::string &ext, Data *data)
             : nodeType(std::move(nodeType)), mExtTokens(splitString(ext)), mData(data)
-        {}
+        {
+            if (mExtTokens.empty())
+                throw InternalError(nullptr, "invalid AST node: " + this->nodeType, InternalError::AST);
+        }
         std::string nodeType;
         std::vector<AstNodePtr> children;
 
@@ -1052,7 +1055,10 @@ Token *clangimport::AstNode::createTokens(TokenList &tokenList)
         return nullptr;
     }
     if (nodeType == EnumConstantDecl) {
-        Token *nameToken = addtoken(tokenList, getSpelling());
+        const std::string name = getSpelling();
+        if (name.empty())
+            throw InternalError(nullptr, "invalid AST node: " + nodeType, InternalError::AST);
+        Token *nameToken = addtoken(tokenList, name);
         auto *scope = const_cast<Scope *>(nameToken->scope());
         scope->enumeratorList.emplace_back(nameToken->scope());
         Enumerator *e = &scope->enumeratorList.back();
@@ -1388,6 +1394,10 @@ Token * clangimport::AstNode::createTokensCall(TokenList &tokenList)
 
 void clangimport::AstNode::createTokensFunctionDecl(TokenList &tokenList)
 {
+    // an address-only node has no name: addtoken("") adds nothing and would return a null back()
+    const std::string name = getSpelling();
+    if (name.empty())
+        throw InternalError(nullptr, "invalid AST node: " + nodeType, InternalError::AST);
     const bool prev = contains(mExtTokens, "prev");
     const bool hasBody = !children.empty() && children.back()->nodeType == CompoundStmt;
     const bool isStatic = contains(mExtTokens, "static");
@@ -1409,11 +1419,14 @@ void clangimport::AstNode::createTokensFunctionDecl(TokenList &tokenList)
     if (mExtTokens.size() > 4 && mExtTokens[1] == "parent")
         addFullScopeNameTokens(tokenList, mData->getScope(mExtTokens[2]));
 
-    Token *nameToken = addtoken(tokenList, getSpelling() + getTemplateParameters());
+    Token *nameToken = addtoken(tokenList, name + getTemplateParameters());
     auto *nestedIn = const_cast<Scope *>(nameToken->scope());
 
     if (prev) {
-        const std::string addr = *(std::find(mExtTokens.cbegin(), mExtTokens.cend(), "prev") + 1);
+        const auto it = std::find(mExtTokens.cbegin(), mExtTokens.cend(), "prev");
+        if (it + 1 == mExtTokens.cend())
+            throw InternalError(nullptr, "invalid AST node: " + nodeType, InternalError::AST);
+        const std::string addr = *(it + 1);
         mData->ref(addr, nameToken);
     }
     if (!nameToken->function()) {
@@ -1460,6 +1473,9 @@ void clangimport::AstNode::createTokensFunctionDecl(TokenList &tokenList)
         AstNodePtr child = children[i];
         if (child->nodeType != ParmVarDecl)
             continue;
+        // there must be a token between the address and the type
+        if (child->mExtTokens.size() < 3)
+            throw InternalError(nullptr, "invalid AST node: " + child->nodeType, InternalError::AST);
         if (tokenList.back() != par1)
             addtoken(tokenList, ",");
         const Type *recordType = addTypeTokens(tokenList, child->mExtTokens.back(), nestedIn);
@@ -1562,6 +1578,9 @@ Token * clangimport::AstNode::createTokensVarDecl(TokenList &tokenList)
     int typeIndex = mExtTokens.size() - 1;
     while (typeIndex > 1 && std::isalpha(mExtTokens[typeIndex][0]))
         typeIndex--;
+    // there must be a name token between the address and the type
+    if (typeIndex < 2)
+        throw InternalError(nullptr, "invalid AST node: " + nodeType, InternalError::AST);
     const std::string type = mExtTokens[typeIndex];
     const std::string name = mExtTokens[typeIndex - 1];
     const Token *startToken = tokenList.back();

test/testclangimport.cpp

@@ -144,6 +144,7 @@ class TestClangImport : public TestFixture {
         TEST_CASE(crash2);
         TEST_CASE(crash3);
         TEST_CASE(nullNodeInvalidLevel);
+        TEST_CASE(invalidExtTokens);
     }
 
     std::string parse(const char clang[]) {
@@ -1421,7 +1422,7 @@ class TestClangImport : public TestFixture {
         // getSpelling() indexed mExtTokens[typeIndex - 1] without a lower-bound
         // check, so a node whose line carries no ext tokens (typeIndex <= 0) read
         // out of bounds.
-        (void)parse("`-RecordDecl "); // don't crash
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-RecordDecl "), AST, "invalid AST node: RecordDecl");
     }
 
     void nullNodeInvalidLevel() {
@@ -1442,6 +1443,29 @@ class TestClangImport : public TestFixture {
                     "  `-CXXRecordDecl 0x2\n"); // don't crash
         (void)parse("`-CXXRecordDecl 0x1"); // don't crash
     }
+
+    void invalidExtTokens() {
+        // nodes without the ext tokens the handlers read must not index out of bounds
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FunctionDecl 0x1 <a.cpp:1:1, col:20> col:5 f 'int ()'\n"
+                                           "  `-CompoundStmt 0x2 <col:9, col:20>\n"
+                                           "    `-ReturnStmt 0x3 <col:11, col:18>\n"
+                                           "      `-IntegerLiteral \n"), AST, "invalid AST node: IntegerLiteral");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-EnumDecl "), AST, "invalid AST node: EnumDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-VarDecl 0x1"), AST, "invalid AST node: VarDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-VarDecl 0x1 'int'"), AST, "invalid AST node: VarDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-VarDecl 0x1 <col:1>"), AST, "invalid AST node: VarDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FieldDecl 0x1 'int'"), AST, "invalid AST node: FieldDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FunctionDecl 0x1 <a.cpp:1:1, col:20> col:5 f 'int ()' prev"), AST, "invalid AST node: FunctionDecl");
+        // address-only declarations: getSpelling() is empty, addtoken() adds nothing and back() is null
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FunctionDecl 0x1"), AST, "invalid AST node: FunctionDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-CXXConstructorDecl 0x1"), AST, "invalid AST node: CXXConstructorDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-CXXMethodDecl 0x1"), AST, "invalid AST node: CXXMethodDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-EnumConstantDecl 0x1"), AST, "invalid AST node: EnumConstantDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FunctionDecl 0x1 <a.cpp:1:1, col:20> col:5 f 'int ()'\n"
+                                           "  `-ParmVarDecl 0x2 <col:1>\n"), AST, "invalid AST node: ParmVarDecl");
+        ASSERT_THROW_INTERNAL_EQUALS(parse("`-FunctionDecl 0x1 <a.cpp:1:1, col:20> col:5 f 'int ()'\n"
+                                           "  `-ParmVarDecl 0x2 'int'\n"), AST, "invalid AST node: ParmVarDecl");
+    }
 };
 
 REGISTER_TEST(TestClangImport)