[all-commits] [llvm/llvm-project] 340be6: [analyzer] Delete `alpha.security.MallocOverflow` ...

[Donát Nagy via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 340be6cb792833b619bf260d6d1f20336008d588
      https://github.com/llvm/llvm-project/commit/340be6cb792833b619bf260d6d1f20336008d588
  Author: Donát Nagy <donat.nagy at ericsson.com>
  Date:   2024-08-14 (Wed, 14 Aug 2024)

  Changed paths:
    M clang/docs/ReleaseNotes.rst
    M clang/docs/analyzer/checkers.rst
    M clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    M clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
    R clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
    R clang/test/Analysis/malloc-overflow.c
    R clang/test/Analysis/malloc-overflow.cpp
    R clang/test/Analysis/malloc-overflow2.c
    M clang/www/analyzer/potential_checkers.html
    M llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn

  Log Message:
  -----------
  [analyzer] Delete `alpha.security.MallocOverflow` (#103059)

...because it is too noisy to be useful right now, and its architecture
is terrible, so it can't act a starting point of future development.

The main problem with this checker is that it tries to do (or at least
fake) path-sensitive analysis without actually using the established
path-sensitive analysis engine.

Instead of actually tracking the symbolic values and the known
constraints on them, this checker blindly gropes the AST and uses
heuristics like "this variable was seen in a comparison operator
expression that is not a loop condition, so it's probably not too large"
(which was improved in a separate commit to at least ignore comparison
operators that appear after the actual `malloc()` call).

This might have been acceptable in 2011 (when this checker was added),
but since then we developed a significantly better standard approach for
analysis and this old relic doesn't deserve to remain in the codebase.

Needless to say, this primitive approach causes lots of false positives
(and presumably false negatives as well), which ensures that this alpha
checker won't be missed by the users.

Moreover, the goals of this checker would be questionable even if it had
a perfect implementation. It's very aggressive to assume that the
argument of malloc can overflow by default (unless the checker sees a
bounds check); and this produces too many false positives -- perhaps
even for an optin checker. It may be possible to eventually create a
useful (and properly path-sensitive) optin checker for these kinds of
suspicious code, but this is a very low priority goal.

Also note that we already have `alpha.security.TaintedAlloc` which
provides more practical heuristics for detecting somewhat similar
"argument of malloc may be too large" vulnerabilities.



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