Skip to content
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.

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

tokenize.cpp: fixed fuzzing crash in setScopeInfo() #6092

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Mar 6, 2024

No description provided.

@@ -129,6 +129,9 @@ void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *a
if (!assignTok->isAssignmentOp() && assignTok->tokType() != Token::eIncDecOp)
return;

if (!assignTok->astOperand1())
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think = must always have both operands, so we should add this to the AST validation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortuantely, at least these cases would require special handling though:

auto f() { [=]() { return data; }(); }
class A {
    virtual void pure()=0;
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So we leave it as is?

Copy link
Collaborator

Choose a reason for hiding this comment

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

New idea: reject = followed by ; and possibly )|} as garbage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not ask me. There's a reason I usually do not get involved into the check related stuff...

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO we should fail early and hard on invalid code. Random nullptr checks here and there probably just enable the next crash down the line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I split this into #6162.

lib/tokenize.cpp Outdated
@@ -2537,7 +2537,7 @@ namespace {
tok1 = tok1->previous();
if (tok1->previous() && (tok1->strAt(-1) == ")" || tok->strAt(-1) == "}")) {
tok1 = tok1->linkAt(-1);
if (Token::Match(tok1->previous(), "throw|noexcept (")) {
if (tok1 && Token::Match(tok1->previous(), "throw|noexcept (")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a check for unlinked )|} somewhere, so I wonder why it doesn't fire.

Copy link
Collaborator

Choose a reason for hiding this comment

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

deleteInvalidTypedef() messes up the links since it runs after createLinks(). Maybeif (!tokOffset || tokOffset->isKeyword()) syntaxError(tok); at tokenize.cpp:1257 is an option.

Copy link
Collaborator Author

@firewave firewave Mar 8, 2024

Choose a reason for hiding this comment

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

The issue does not appear to be fully fixed by my change. Will look into that first and then look into your comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This required a few more checks down the line.

Copy link
Collaborator

@chrchr-github chrchr-github Mar 27, 2024

Choose a reason for hiding this comment

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

Maybeif (!tokOffset || tokOffset->isKeyword()) syntaxError(tok); at tokenize.cpp:1257 is an option.

isControlFlowKeyword() seems to work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What line is that now? Or do you want to take over in a new PR?

Copy link
Collaborator

@chrchr-github chrchr-github Apr 2, 2024

Choose a reason for hiding this comment

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

tokenize.cpp:1257 is still correct (// check for invalid input),

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is line 1255 in my tree...but that works.

@firewave firewave marked this pull request as draft March 6, 2024 20:56
@firewave firewave changed the title fixed some more fuzzing crashes fixed fuzzing crash in setScopeInfo() Mar 20, 2024
@firewave firewave changed the title fixed fuzzing crash in setScopeInfo() tokenize.cpp: fixed fuzzing crash in setScopeInfo() Mar 20, 2024
@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

With Clang I know get a segmentation fault even with ASAN and UBSAN enabled. Using GCC I get

/home/user/CLionProjects/cppcheck/lib/tokenize.cpp:2539:52: runtime error: member call on null pointer of type 'struct Token'
    #0 0x636010e8b660 in setScopeInfo /home/user/CLionProjects/cppcheck/lib/tokenize.cpp:2539
    #1 0x636010e939ff in Tokenizer::simplifyUsing() /home/user/CLionProjects/cppcheck/lib/tokenize.cpp:2902
    #2 0x636010eccf0a in Tokenizer::simplifyTokenList1(char const*) /home/user/CLionProjects/cppcheck/lib/tokenize.cpp:5676
    #3 0x636010e9bc65 in Tokenizer::simplifyTokens1(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/user/CLionProjects/cppcheck/lib/tokenize.cpp:3373
    #4 0x636011c46462 in CppCheck::checkFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::istream*) /home/user/CLionProjects/cppcheck/lib/cppcheck.cpp:930
    #5 0x636011c3361e in CppCheck::check(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/user/CLionProjects/cppcheck/lib/cppcheck.cpp:563
    #6 0x6360108cd0fd in SingleExecutor::check() /home/user/CLionProjects/cppcheck/cli/singleexecutor.cpp:53
    #7 0x63601079fbad in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck/cli/cppcheckexecutor.cpp:279
    #8 0x63601079d34f in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck/cli/cppcheckexecutor.cpp:218
    #9 0x63601079cfa3 in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck/cli/cppcheckexecutor.cpp:204
    #10 0x636012573d13 in main /home/user/CLionProjects/cppcheck/cli/main.cpp:91
    #11 0x7f413ea43ccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #12 0x7f413ea43d89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #13 0x6360106bb534 in _start (/home/user/CLionProjects/cppcheck/cmake-build-debug-gcc-asan-ubsan/bin/cppcheck+0x4f39534) (BuildId: 898fa100dc84a3116411278d71aeba570e5f512e)

@firewave firewave force-pushed the fuzz-crash2 branch 3 times, most recently from 0d0deb2 to 12aff86 Compare April 2, 2024 20:27
@firewave firewave marked this pull request as ready for review April 2, 2024 20:37
/home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2555:31: runtime error: member call on null pointer of type 'Token'
    #0 0x62508946d2b4 in (anonymous namespace)::setScopeInfo(Token*, (anonymous namespace)::ScopeInfo3**, bool) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2555:31
    #1 0x6250894629b6 in Tokenizer::simplifyUsing() /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2903:17
    danmar#2 0x62508947d6c5 in Tokenizer::simplifyTokenList1(char const*) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:5674:12
    danmar#3 0x625089476021 in Tokenizer::simplifyTokens1(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:3374:14
    danmar#4 0x625089f54ca1 in CppCheck::checkFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::istream*) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:937:32
    danmar#5 0x625089f44014 in CppCheck::check(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:564:12
    danmar#6 0x62508904544b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29
    danmar#7 0x625088f7f97b in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:279:32
    danmar#8 0x625088f7e74d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:218:12
    danmar#9 0x625088f7d68a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:204:21
    danmar#10 0x62508a68c327 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21
    danmar#11 0x761e7f21eccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#12 0x761e7f21ed89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#13 0x625088db79f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6b9f4) (BuildId: c4d5a113239183a4005a1a2662be02131ded6024)

fixed fuzzing crash in `setScopeInfo()`

/home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2548:51: runtime error: member call on null pointer of type 'Token'
    #0 0x5d3809ee7015 in (anonymous namespace)::setScopeInfo(Token*, (anonymous namespace)::ScopeInfo3**, bool) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2548:51
    #1 0x5d3809edc9b6 in Tokenizer::simplifyUsing() /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2903:17
    danmar#2 0x5d3809ef76a5 in Tokenizer::simplifyTokenList1(char const*) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:5674:12
    danmar#3 0x5d3809ef0001 in Tokenizer::simplifyTokens1(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:3374:14
    danmar#4 0x5d380a9cec81 in CppCheck::checkFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::istream*) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:937:32
    danmar#5 0x5d380a9bdff4 in CppCheck::check(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:564:12
    danmar#6 0x5d3809abf44b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29
    danmar#7 0x5d38099f997b in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:279:32
    danmar#8 0x5d38099f874d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:218:12
    danmar#9 0x5d38099f768a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:204:21
    danmar#10 0x5d380b106307 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21
    danmar#11 0x7fe30271fccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#12 0x7fe30271fd89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#13 0x5d38098319f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6b9f4) (BuildId: 0e9bdb7f1c43e507f2fcb4d97a15b0f529a13c20)

fixed fuzzing crash in `setScopeInfo()`

/home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2540:44: runtime error: member call on null pointer of type 'Token'
    #0 0x5ec589e5f40f in (anonymous namespace)::setScopeInfo(Token*, (anonymous namespace)::ScopeInfo3**, bool) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2540:44
    #1 0x5ec589e5505d in Tokenizer::simplifyUsing() /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:2900:17
    danmar#2 0x5ec589e6fd25 in Tokenizer::simplifyTokenList1(char const*) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:5671:12
    danmar#3 0x5ec589e68681 in Tokenizer::simplifyTokens1(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/tokenize.cpp:3371:14
    danmar#4 0x5ec58a9497ca in CppCheck::checkFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::istream*) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:925:32
    danmar#5 0x5ec58a938a97 in CppCheck::check(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:556:12
    danmar#6 0x5ec589a36f2b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29
    danmar#7 0x5ec5899718dc in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:277:32
    danmar#8 0x5ec58997073d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:216:12
    danmar#9 0x5ec58996f67a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:202:21
    danmar#10 0x5ec58b0802b7 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21
    danmar#11 0x79b410843ccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#12 0x79b410843d89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    danmar#13 0x5ec5897aa9f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6e9f4) (BuildId: 5c2986a23a9dee600c328566a7967a7eba8652c9)

Co-authored-by: chrchr-github <[email protected]>
@chrchr-github chrchr-github merged commit f722f08 into danmar:main Apr 2, 2024
64 checks passed
@firewave firewave deleted the fuzz-crash2 branch April 2, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants