[all-commits] [llvm/llvm-project] fb299c: [analyzer] Make recognition of hardened __FOO_chk ...

[NagyDonat via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: fb299cae5167f63933df45979e3e9de97fca1b8f
      https://github.com/llvm/llvm-project/commit/fb299cae5167f63933df45979e3e9de97fca1b8f
  Author: NagyDonat <donat.nagy at ericsson.com>
  Date:   2024-04-05 (Fri, 05 Apr 2024)

  Changed paths:
    M clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
    M clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
    M clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    M clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
    M clang/lib/StaticAnalyzer/Core/CallDescription.cpp
    M clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
    M clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

  Log Message:
  -----------
  [analyzer] Make recognition of hardened __FOO_chk functions explicit (#86536)

In builds that use source hardening (-D_FORTIFY_SOURCE), many standard
functions are implemented as macros that expand to calls of hardened
functions that take one additional argument compared to the "usual"
variant and perform additional input validation. For example, a `memcpy`
call may expand to `__memcpy_chk()` or `__builtin___memcpy_chk()`.

Before this commit, `CallDescription`s created with the matching mode
`CDM::CLibrary` automatically matched these hardened variants (in a
addition to the "usual" function) with a fairly lenient heuristic.

Unfortunately this heuristic meant that the `CLibrary` matching mode was
only usable by checkers that were prepared to handle matches with an
unusual number of arguments.

This commit limits the recognition of the hardened functions to a
separate matching mode `CDM::CLibraryMaybeHardened` and applies this
mode for functions that have hardened variants and were previously
recognized with `CDM::CLibrary`.

This way checkers that are prepared to handle the hardened variants will
be able to detect them easily; while other checkers can simply use
`CDM::CLibrary` for matching C library functions (and they won't
encounter surprising argument counts).

The initial motivation for refactoring this area was that previously
`CDM::CLibrary` accepted calls with more arguments/parameters than the
expected number, so I wasn't able to use it for `malloc` without
accidentally matching calls to the 3-argument BSD kernel malloc.

After this commit this "may have more args/params" logic will only
activate when we're actually matching a hardened variant function (in
`CDM::CLibraryMaybeHardened` mode). The recognition of "sprintf()" and
"snprintf()" in CStringChecker was refactored, because previously it was
abusing the behavior that extra arguments are accepted even if the
matched function is not a hardened variant.

This commit also fixes the oversight that the old code would've
recognized e.g. `__wmemcpy_chk` as a hardened variant of `memcpy`.

After this commit I'm planning to create several follow-up commits that
ensure that checkers looking for C library functions use `CDM::CLibrary`
as a "sane default" matching mode.

This commit is not truly NFC (it eliminates some buggy corner cases),
but it does not intentionally modify the behavior of CSA on real-world
non-crazy code.

As a minor unrelated change I'm eliminating the argument/variable
"IsBuiltin" from the evalSprintf function family in CStringChecker,
because it was completely unused.

---------

Co-authored-by: Balazs Benics <benicsbalazs at gmail.com>



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications