checkautovariables.cpp: fixed fuzzing crash in `isArrayArg`

[firewave]

/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]

Pulled out of #6089 as per https://github.com/danmar/cppcheck/pull/6089#discussion_r1520521540.

[chrchr-github]

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]

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]

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

This was added in d45f5c94cba8c58a9107ae3ffe8ad72fc83d8792 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]

See https://github.com/danmar/cppcheck/pull/6118 I think the impact on performance should be negligible (linear in the number of variables).

[firewave]

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]

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..

[firewave]

Closing as this is superseded by #6118.

[firewave]

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

[firewave]

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)

[chrchr-github]

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

(; should be a syntaxError

[firewave]

Closing again - this time in favor of #6197.