checkautovariables.cpp: fixed fuzzing crash in `isArrayArg` by firewave · Pull Request #6116 · danmar/cppcheck

firewave · 2024-03-11T23:13:14Z

/home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105: runtime error: member access within null pointer of type 'const Scope'
    #0 0x58828480410b in isArrayArg(Token const*, Settings const*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105
    #1 0x588284801124 in CheckAutoVariables::autoVariables() /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:293:50
    #2 0x58828481985f in CheckAutoVariables::runChecks(Tokenizer const&, ErrorLogger*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.h:62:28
    #3 0x588284e7d5ed in CppCheck::checkNormalTokens(Tokenizer const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:1124:20
    #4 0x588284e965cb 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:957:17
    #5 0x588284e83bb7 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
    #6 0x588283f81f2b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29
    #7 0x588283ebc8dc in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:277:32
    #8 0x588283ebb73d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:216:12
    #9 0x588283eba67a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:202:21
    #10 0x5882855cb3d7 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21
    #11 0x7d6f6a355ccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #12 0x7d6f6a355d89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #13 0x588283cf59f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6d9f4) (BuildId: 88417f372233fa3e0202193c0dd69f6cfca2ff4c)

firewave · 2024-03-11T23:13:54Z

Pulled out of #6089 as per #6089 (comment).

chrchr-github · 2024-03-12T09:31:05Z

As it turns out, we already have SymbolDatabase::validateVariables(), which checks for variables without scope. I wonder why this is not caught there?
Edit: That's why: 🤦

    // TODO
    //validateVariables();

chrchr-github · 2024-03-12T12:57:00Z

This seems to work after adjusting one test case:

namespace {
    const Function* getFunctionForArgumentvariable(const Variable * const var, const std::vector<const Scope *>& functionScopes)
    {
        const std::size_t functions = functionScopes.size();
        for (std::size_t i = 0; i < functions; ++i) {
            const Scope* const scope = functionScopes[i];
            const Function* const function = scope->function;
            if (function) {
                for (std::size_t arg=0; arg < function->argCount(); ++arg) {
                    if (var==function->getArgumentVar(arg))
                        return function;
                }
            }
        }
        if (const Scope* scope = var->nameToken()->scope()) {
            auto it = std::find_if(scope->functionList.begin(), scope->functionList.end(), [&](const Function& function) {
                for (std::size_t arg = 0; arg < function.argCount(); ++arg) {
                    if (var == function.getArgumentVar(arg))
                        return true;
                }
                return false;
            });
            if (it != scope->functionList.end())
                return &*it;
        }
        return nullptr;
    }
}

void SymbolDatabase::validateVariables() const
{
    for (std::vector<const Variable *>::const_iterator iter = mVariableList.cbegin(); iter!=mVariableList.cend(); ++iter) {
        const Variable * const var = *iter;
        if (var) {
            if (!var->scope()) {
                const Function* function = getFunctionForArgumentvariable(var, functionScopes);
                if (!var->isArgument() || (!function || function->hasBody())) {
                    throw InternalError(var->nameToken(), "Analysis failed (variable without scope). If the code is valid then please report this failure.", InternalError::INTERNAL);
                }
            }
        }
    }
}

void SymbolDatabase::validate() const
{
    if (mSettings.debugwarnings) {
        validateExecutableScopes();
    }
    validateVariables();
}

firewave · 2024-03-12T14:15:51Z

Feel free to open your own PR in favor of this. The individual validate functions should be made private.

This was added in d45f5c9 over 8(!!!) years ago and was never enabled.

We also need to profile to make sure it doesn't affect the performance. Maybe we need to put it behind --debug-warnings later on.

I am also curious why this didn't show up in the unusedFunction check.

chrchr-github · 2024-03-12T14:35:38Z

See #6118
I think the impact on performance should be negligible (linear in the number of variables).

firewave · 2024-03-12T14:46:07Z

I think the impact on performance should be negligible (linear in the number of variables).

As this is in the Tokenizer we might not need to explicitly profile at all. You can just look at the Ir count in the callgrind step of the selfcheck job and compare that with the latest run on the main branch.

danmar · 2024-03-13T10:59:59Z

wait.. it's not valid syntax.

I don't think our checkautovariables should handle this garbage.

you can add a syntax checking in the tokenizer. however .. we do want to handle non-standard syntax so you need to be careful when writing syntax checkers you could break some valid code..

danmar

syntax error

firewave · 2024-03-18T08:49:05Z

Closing as this is superseded by #6118.

firewave · 2024-03-18T17:10:20Z

I am still seeing a crash with this trace. Will investigate later.

firewave · 2024-03-18T17:21:30Z

i a;u n(;a[]),n(){a[]=0}

/home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105: runtime error: member access within null pointer of type 'const Scope'
    #0 0x5cd9c01906fb in isArrayArg(Token const*, Settings const*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105
    #1 0x5cd9c018d8f4 in CheckAutoVariables::autoVariables() /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:293:50
    #2 0x5cd9c01a560f in CheckAutoVariables::runChecks(Tokenizer const&, ErrorLogger*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.h:62:28
    #3 0x5cd9c08088bd in CppCheck::checkNormalTokens(Tokenizer const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:1132:20
    #4 0x5cd9c082193b 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:965:17
    #5 0x5cd9c080ef64 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
    #6 0x5cd9bf91044b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29
    #7 0x5cd9bf84a97b in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:279:32
    #8 0x5cd9bf84974d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:218:12
    #9 0x5cd9bf84868a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:204:21
    #10 0x5cd9c0f56eb7 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21
    #11 0x7e6d76e1eccf  (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #12 0x7e6d76e1ed89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2)
    #13 0x5cd9bf6829f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6b9f4) (BuildId: 57a36609553096fb65d63bdeae23688115ebef1e)

/home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105: runtime error: member access within null pointer of type 'const Scope' #0 0x5cd9c01906fb in isArrayArg(Token const*, Settings const*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:61:105 #1 0x5cd9c018d8f4 in CheckAutoVariables::autoVariables() /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.cpp:293:50 danmar#2 0x5cd9c01a560f in CheckAutoVariables::runChecks(Tokenizer const&, ErrorLogger*) /home/user/CLionProjects/cppcheck-rider/lib/checkautovariables.h:62:28 danmar#3 0x5cd9c08088bd in CppCheck::checkNormalTokens(Tokenizer const&) /home/user/CLionProjects/cppcheck-rider/lib/cppcheck.cpp:1132:20 danmar#4 0x5cd9c082193b 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:965:17 danmar#5 0x5cd9c080ef64 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 0x5cd9bf91044b in SingleExecutor::check() /home/user/CLionProjects/cppcheck-rider/cli/singleexecutor.cpp:53:29 danmar#7 0x5cd9bf84a97b in CppCheckExecutor::check_internal(Settings const&) const /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:279:32 danmar#8 0x5cd9bf84974d in CppCheckExecutor::check_wrapper(Settings const&) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:218:12 danmar#9 0x5cd9bf84868a in CppCheckExecutor::check(int, char const* const*) /home/user/CLionProjects/cppcheck-rider/cli/cppcheckexecutor.cpp:204:21 danmar#10 0x5cd9c0f56eb7 in main /home/user/CLionProjects/cppcheck-rider/cli/main.cpp:91:21 danmar#11 0x7e6d76e1eccf (/usr/lib/libc.so.6+0x29ccf) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2) danmar#12 0x7e6d76e1ed89 in __libc_start_main (/usr/lib/libc.so.6+0x29d89) (BuildId: 0865c4b9ba13e0094e8b45b78dfc7a2971f536d2) danmar#13 0x5cd9bf6829f4 in _start (/home/user/CLionProjects/cppcheck-rider/cmake-build-debug-clang-asan-ubsan/bin/cppcheck+0xf6b9f4) (BuildId: 57a36609553096fb65d63bdeae23688115ebef1e)

chrchr-github · 2024-03-22T13:50:41Z

i a;u n(;a[]),n(){a[]=0}

(; should be a syntaxError

firewave · 2024-03-28T09:32:30Z

Closing again - this time in favor of #6197.

Poached from #6116

firewave mentioned this pull request Mar 11, 2024

fixed fuzzing crashes #6089

Merged

danmar requested changes Mar 13, 2024

View reviewed changes

firewave closed this Mar 18, 2024

firewave deleted the fuzz-crash-x branch March 18, 2024 08:49

firewave restored the fuzz-crash-x branch March 18, 2024 17:09

firewave reopened this Mar 18, 2024

firewave force-pushed the fuzz-crash-x branch from 02b1fa6 to 5da52ea Compare March 18, 2024 17:23

chrchr-github mentioned this pull request Mar 27, 2024

Fix fuzzing crash: reject semicolon within parentheses #6197

Merged

firewave closed this Mar 28, 2024

firewave deleted the fuzz-crash-x branch March 28, 2024 09:32

orbitcowboy pushed a commit that referenced this pull request Mar 29, 2024

Fix fuzzing crash: reject semicolon within parentheses (#6197)

356b2bd

Poached from #6116

Conversation

firewave commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firewave commented Mar 11, 2024

Uh oh!

chrchr-github commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrchr-github commented Mar 12, 2024

Uh oh!

firewave commented Mar 12, 2024

Uh oh!

chrchr-github commented Mar 12, 2024

Uh oh!

firewave commented Mar 12, 2024

Uh oh!

danmar commented Mar 13, 2024

Uh oh!

danmar left a comment

Choose a reason for hiding this comment

Uh oh!

firewave commented Mar 18, 2024

Uh oh!

firewave commented Mar 18, 2024

Uh oh!

firewave commented Mar 18, 2024

Uh oh!

chrchr-github commented Mar 22, 2024

Uh oh!

firewave commented Mar 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

firewave commented Mar 11, 2024 •

edited

Loading

chrchr-github commented Mar 12, 2024 •

edited

Loading