This commit moves the **alpha.security.taint.TaintPropagation** and
**alpha.security.taint.GenericTaint** checkers to the **optin.taint**
optional package.
These checkers were stabilized and improved by recent commits thus
they are ready for production use.
A new optional checker (optin.taint.TaintedAlloc) will warn if a memory
allocation function (malloc, calloc, realloc, alloca, operator new[]) is
called with a tainted (attacker controlled) size parameter.
A large, maliciously set size value can trigger memory exhaustion. To
get this warning, the alpha.security.taint.TaintPropagation checker also
needs to be switched on.
The warning will only be emitted, if the analyzer cannot prove that the
size is below reasonable bounds (<SIZE_MAX/4).
This commit ensures that the `CallDescription`s in `MallocChecker` are
matched with the mode `CDM::CLibrary`, so:
- they don't match methods or functions within user-defined namespaces;
- they also match builtin variants of these functions (if any), so the
checker can model `__builtin_alloca()` like `alloca()`.
This change fixes https://github.com/llvm/llvm-project/issues/81597. New
tests were added to verify that `std::malloc` and `std::free` (from
`<cstdlib>`) are modeled, but a method that's named e.g. `free` isn't
confused with the memory release function.
The responsibility for modeling `__builtin_alloca` and
`__builtin_alloca_with_align` was moved from `BuiltinFunctionChecker` to
`MallocChecker`, to avoid buggy interactions between the checkers and
ensure that the builtin and non-builtin variants are handled by exactly
the same logic.
This change might be a step backwards for the users who don't have
`unix.Malloc` enabled; but I suspect that `__builtin_alloca()` is so
rare that it would be a waste of time to implement backwards
compatibility for them.
There were several test files that relied on `__builtin_alloca()` calls
to get an `AllocaRegion`, these were modified to enable `unix.Malloc`.
One of these files (cxx-uninitialized-object-ptr-ref.cpp) had some tests
that relied on the fact that `malloc()` was treated as a "black box" in
them, these were updated to use `calloc()` (to get initialized memory)
and `free()` (to avoid memory leak reports).
While I was developing this change, I found a very suspicious assert in
`MallocChecker`. As it isn't blocking the goals of this commit, I just
marked it with a FIXME, but I'll try to investigate and fix it in a
follow-up change.
We have a new policy in place making links to private resources
something we try to avoid in source and test files. Normally, we'd
organically switch to the new policy rather than make a sweeping change
across a project. However, Clang is in a somewhat special circumstance
currently: recently, I've had several new contributors run into rdar
links around test code which their patch was changing the behavior of.
This turns out to be a surprisingly bad experience, especially for
newer folks, for a handful of reasons: not understanding what the link
is and feeling intimidated by it, wondering whether their changes are
actually breaking something important to a downstream in some way,
having to hunt down strangers not involved with the patch to impose on
them for help, accidental pressure from asking for potentially private
IP to be made public, etc. Because folks run into these links entirely
by chance (through fixing bugs or working on new features), there's not
really a set of problematic links to focus on -- all of the links have
basically the same potential for causing these problems. As a result,
this is an omnibus patch to remove all such links.
This was not a mechanical change; it was done by manually searching for
rdar, radar, radr, and other variants to find all the various
problematic links. From there, I tried to retain or reword the
surrounding comments so that we would lose as little context as
possible. However, because most links were just a plain link with no
supporting context, the majority of the changes are simple removals.
Differential Review: https://reviews.llvm.org/D158071
The program point created by the checker, even if it is an error node,
might not be the same as the name under which the report is emitted.
Make sure we're checking the name of the checker, because thats what
we're silencing after all.
Differential Revision: https://reviews.llvm.org/D102683
https://bugs.llvm.org/show_bug.cgi?id=46253
This is an obvious hack because realloc isn't any more affected than other
functions modeled by MallocChecker (or any user of CallDescription really),
but the nice solution will take some time to implement.
Differential Revision: https://reviews.llvm.org/D81745
Summary:
The kernel kmalloc function may return a constant value ZERO_SIZE_PTR
if a zero-sized block is allocated. This special value is allowed to
be passed to kfree and should produce no warning.
This is a simple version but should be no problem. The macro is always
detected independent of if this is a kernel source code or any other
code.
Reviewers: Szelethus, martong
Reviewed By: Szelethus, martong
Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76830
Writing stuff into an argument variable is usually equivalent to writing stuff
to a local variable: it will have no effect outside of the function.
There's an important exception from this rule: if the argument variable has
a non-trivial destructor, the destructor would be invoked on
the parent stack frame, exposing contents of the otherwise dead
argument variable to the caller.
If such argument is the last place where a pointer is stored before the function
exits and the function is the one we've started our analysis from (i.e., we have
no caller context for it), we currently diagnose a leak. This is incorrect
because the destructor of the argument still has access to the pointer.
The destructor may deallocate the pointer or even pass it further.
Treat writes into such argument regions as "escapes" instead, suppressing
spurious memory leak reports but not messing with dead symbol removal.
Differential Revision: https://reviews.llvm.org/D60112
llvm-svn: 358321
In order to provide more test coverage for inlined operator new(), add more
run-lines to existing test cases, which would trigger our fake header
to provide a body for operator new(). Most of the code should still behave
reasonably. When behavior intentionally changes, #ifs are provided.
Differential Revision: https://reviews.llvm.org/D42221
llvm-svn: 323376
Previously, this was scattered across Environment (literal expressions),
ExprEngine (default arguments), and RegionStore (global constants). The
former special-cased several kinds of simple constant expressions, while
the latter two deferred to the AST's constant evaluator.
Now, these are all unified as SValBuilder::getConstantVal(). To keep
Environment fast, the special cases for simple constant expressions have
been left in, but the main benefits are that (a) unusual constants like
ObjCStringLiterals now work as default arguments and global constant
initializers, and (b) we're not duplicating code between ExprEngine and
RegionStore.
This actually caught a bug in our test suite, which is awesome: we stop
tracking allocated memory if it's passed as an argument along with some
kind of callback, but not if the callback is 0. We were testing this in
a case where the callback parameter had a default value, but that value
was 0. After this change, the analyzer now (correctly) flags that as a
leak!
<rdar://problem/13773117>
llvm-svn: 180894
Refactor invalidateRegions to take SVals instead of Regions as input and teach RegionStore
about processing LazyCompoundVal as a top-level “escaping” value.
This addresses several false positives that get triggered by the NewDelete checker, but the
underlying issue is reproducible with other checkers as well (for example, MallocChecker).
llvm-svn: 178518
This is a "quick fix".
The underlining issue is that when a const pointer to a struct is passed
into a function, we do not invalidate the pointer fields. This results
in false positives that are common in C++ (since copy constructors are
prevalent). (Silences two llvm false positives.)
llvm-svn: 174468
This fixes a few cases where we'd emit path notes like this:
+---+
1| v
p = malloc(len);
^ |2
+---+
In general this should make path notes more consistent and more correct,
especially in cases where the leak happens on the false branch of an if
that jumps directly to the end of the function. There are a couple places
where the leak is reported farther away from the cause; these are usually
cases where there are several levels of nested braces before the end of
the function. This still matches our current behavior for when there /is/
a statement after all the braces, though.
llvm-svn: 168070
While destructors will continue to not be inlined (unless the analyzer
config option 'c++-inlining' is set to 'destructors'), leaving them out
of the CFG is an incomplete model of the behavior of an object, and
can cause false positive warnings (like PR13751, now working).
Destructors for temporaries are still not on by default, since
(a) we haven't actually checked this code to be sure it's fully correct
(in particular, we probably need to be very careful with regard to
lifetime-extension when a temporary is bound to a reference,
C++11 [class.temporary]p5), and
(b) ExprEngine doesn't actually do anything when it sees a temporary
destructor in the CFG -- not even invalidate the object region.
To enable temporary destructors, set the 'cfg-temporary-dtors' analyzer
config option to '1'. The old -cfg-add-implicit-dtors cc1 option, which
controlled all implicit destructors, has been removed.
llvm-svn: 163264
