Skip to content

Fix use-after-free crash when using --clang #5367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 3 commits into from
Aug 28, 2023
Merged

Fix use-after-free crash when using --clang #5367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
757ea68
Fix use-after-free crash when using --clang
chrchr-github Aug 24, 2023
0eb992d
Add suppression
chrchr-github Aug 24, 2023
5806725
Add test
chrchr-github Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/checkvaarg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ void CheckVaarg::va_start_argument()
if (var && var->isReference())
referenceAs_va_start_error(param2, var->name());
if (var && var->index() + 2 < function->argCount() && printWarnings) {
wrongParameterTo_va_start_error(tok, var->name(), function->argumentList[function->argumentList.size()-2].name());
auto it = function->argumentList.end();
std::advance(it, -2);
wrongParameterTo_va_start_error(tok, var->name(), it->name()); // cppcheck-suppress derefInvalidIterator // FP due to isVariableChangedByFunctionCall()
}
tok = tok->linkAt(1);
}
Expand Down
1 change: 0 additions & 1 deletion lib/clangimport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,6 @@ void clangimport::AstNode::createTokensFunctionDecl(TokenList *tokenList)
function->nestedIn = nestedIn;
function->argDef = par1;
// Function arguments
function->argumentList.reserve(children.size());
for (int i = 0; i < children.size(); ++i) {
AstNodePtr child = children[i];
if (child->nodeType != ParmVarDecl)
Expand Down
7 changes: 5 additions & 2 deletions lib/symboldatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4427,8 +4427,11 @@ const Function * Function::getOverriddenFunctionRecursive(const ::Type* baseType

const Variable* Function::getArgumentVar(nonneg int num) const
{
if (num < argumentList.size())
return &argumentList[num];
if (num < argumentList.size()) {
auto it = argumentList.begin();
std::advance(it, num);
Copy link
Copy Markdown
Collaborator

@danmar danmar Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand.. is this safer or faster in some way?

Copy link
Copy Markdown
Collaborator Author

@chrchr-github chrchr-github Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you do it with a std::list?

Copy link
Copy Markdown
Collaborator

@danmar danmar Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok understood.. it seems unfortunate to use a std::list but not sure how to solve it properly with the vector. :-(

return &*it;
}
return nullptr;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/symboldatabase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ class CPPCHECKLIB Function {
const ::Type* retType{}; ///< function return type
const Scope* functionScope{}; ///< scope of function body
const Scope* nestedIn{}; ///< Scope the function is declared in
std::vector<Variable> argumentList; ///< argument list
std::list<Variable> argumentList; ///< argument list, must remain list due to clangimport usage!
nonneg int initArgCount{}; ///< number of args with default values
Type type = eFunction; ///< constructor, destructor, ...
const Token* noexceptArg{}; ///< noexcept token
Expand Down
48 changes: 48 additions & 0 deletions test/testclangimport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class TestClangImport : public TestFixture {

TEST_CASE(valueType1);
TEST_CASE(valueType2);

TEST_CASE(crash);
}

std::string parse(const char clang[]) {
Expand Down Expand Up @@ -1314,6 +1316,52 @@ class TestClangImport : public TestFixture {
ASSERT(!!tok->valueType());
ASSERT_EQUALS("const signed char *", tok->valueType()->str());
}

void crash() {
const char* clang = "TranslationUnitDecl 0x56037914f998 <<invalid sloc>> <invalid sloc>\n"
"|-TypedefDecl 0x560379150200 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'\n"
"| `-BuiltinType 0x56037914ff60 '__int128'\n"
"|-TypedefDecl 0x560379150270 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'\n"
"| `-BuiltinType 0x56037914ff80 'unsigned __int128'\n"
"|-TypedefDecl 0x5603791505e8 <<invalid sloc>> <invalid sloc> implicit __NSConstantString '__NSConstantString_tag'\n"
"| `-RecordType 0x560379150360 '__NSConstantString_tag'\n"
"| `-CXXRecord 0x5603791502c8 '__NSConstantString_tag'\n"
"|-TypedefDecl 0x560379150680 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'\n"
"| `-PointerType 0x560379150640 'char *'\n"
"| `-BuiltinType 0x56037914fa40 'char'\n"
"|-TypedefDecl 0x5603791968f8 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list '__va_list_tag[1]'\n"
"| `-ConstantArrayType 0x5603791968a0 '__va_list_tag[1]' 1 \n"
"| `-RecordType 0x560379150770 '__va_list_tag'\n"
"| `-CXXRecord 0x5603791506d8 '__va_list_tag'\n"
"|-ClassTemplateDecl 0x560379196b58 <test1.cpp:1:1, col:50> col:37 A\n"
"| |-TemplateTypeParmDecl 0x560379196950 <col:11> col:19 typename depth 0 index 0\n"
"| |-TemplateTypeParmDecl 0x5603791969f8 <col:21> col:29 typename depth 0 index 1\n"
"| `-CXXRecordDecl 0x560379196ac8 <col:31, col:50> col:37 class A definition\n"
"| |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init\n"
"| | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr\n"
"| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param\n"
"| | |-MoveConstructor exists simple trivial needs_implicit\n"
"| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param\n"
"| | |-MoveAssignment exists simple trivial needs_implicit\n"
"| | `-Destructor simple irrelevant trivial needs_implicit\n"
"| |-CXXRecordDecl 0x560379196de0 <col:31, col:37> col:37 implicit referenced class A\n"
"| `-CXXRecordDecl 0x560379196e70 <col:41, col:47> col:47 class b\n"
"|-CXXRecordDecl 0x560379197110 parent 0x560379196ac8 prev 0x560379196e70 <line:2:1, col:63> col:50 class b definition\n"
"| |-DefinitionData empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init\n"
"| | |-DefaultConstructor defaulted_is_constexpr\n"
"| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param\n"
"| | |-MoveConstructor exists simple trivial needs_implicit\n"
"| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param\n"
"| | |-MoveAssignment exists simple trivial needs_implicit\n"
"| | `-Destructor simple irrelevant trivial needs_implicit\n"
"| |-CXXRecordDecl 0x560379197250 <col:35, col:50> col:50 implicit referenced class b\n"
"| `-CXXConstructorDecl 0x5603791974b8 <col:54, col:60> col:54 b 'void (A<type-parameter-0-0, type-parameter-0-1> &)'\n"
"| `-ParmVarDecl 0x560379197380 <col:56, col:59> col:59 a 'A<type-parameter-0-0, type-parameter-0-1> &'\n"
"`-CXXConstructorDecl 0x5603791b5600 parent 0x560379197110 prev 0x5603791974b8 <line:3:1, col:55> col:47 b 'void (A<type-parameter-0-0, type-parameter-0-1> &)'\n"
" |-ParmVarDecl 0x5603791b5570 <col:49, col:51> col:52 'A<type-parameter-0-0, type-parameter-0-1> &'\n"
" `-CompoundStmt 0x5603791b5700 <col:54, col:55>\n";
parse(clang); // don't crash
}
};

REGISTER_TEST(TestClangImport)