[clang] [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker (PR #67663)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 28 08:10:18 PDT 2023
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>,
Endre =?utf-8?q?Fülöp?= <endre.fulop at sigmatechnology.se>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/67663/clang at github.com>
================
@@ -94,23 +119,40 @@ REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const FunctionDecl *,
void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
CheckerContext &C) const {
StringRef FunctionName = Call.getCalleeIdentifier()->getName();
- ProgramStateRef State = C.getState();
- const MemRegion *SymbolicEnvPtrRegion = State->get<EnvPtrRegion>();
- if (!SymbolicEnvPtrRegion)
- return;
-
- State = State->add<InvalidMemoryRegions>(SymbolicEnvPtrRegion);
- const NoteTag *Note =
- C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
- PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(SymbolicEnvPtrRegion))
- return;
- Out << '\'' << FunctionName
- << "' call may invalidate the environment parameter of 'main'";
- });
+ auto PlaceInvalidationNote = [&C, FunctionName](ProgramStateRef State,
+ const MemRegion *Region,
+ StringRef Message,
+ ExplodedNode *Pred) {
+ State = State->add<InvalidMemoryRegions>(Region);
+
+ // Make copy of string data for the time when notes are *actually* created.
+ const NoteTag *Note =
+ C.getNoteTag([Region, FunctionName = std::string{FunctionName},
+ Message = std::string{Message}](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ if (!BR.isInteresting(Region) ||
+ &BR.getBugType() != InvalidPtrBugType)
+ return;
+ Out << '\'' << FunctionName << "' " << Message;
+ BR.markNotInteresting(Region);
+ });
+ return C.addTransition(State, Pred, Note);
+ };
- C.addTransition(State, Note);
+ ProgramStateRef State = C.getState();
+ ExplodedNode *CurrentChainEnd = C.getPredecessor();
+
+ if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ State, MainEnvPtr,
+ "call may invalidate the environment parameter of 'main'",
+ CurrentChainEnd);
+
+ for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ State, EnvPtr, "call may invalidate the environment returned by getenv",
+ CurrentChainEnd);
----------------
DonatNagyE wrote:
On the [old Phabricator review](https://reviews.llvm.org/D154603) of these modifications, @steakhal wrote that
> I'd prefer if we wouldn't put N separate NoteTags, but rather iterate over this set of regions inside the NoteTag.
That way here you don't need the loop and play with Pred nodes.
I agree with his suggestion: try to refactor this code and create a single note tag constructed from a "smart" lambda that captures both the `MainEnvPtr` and the array `State->get<GetenvEnvPtrRegions>()`, checks the bug type and if it corresponds to this checker, then checks each of the captured `MemRegion`s for interestingness and produces the corresponding message if it finds an interesting one.
In the common case there will be just one interesting `MemRegion`, but think a bit about corner cases where multiple invalidated regions are referenced at the same time and handle them somehow if you think that they can appear.
https://github.com/llvm/llvm-project/pull/67663
More information about the cfe-commits
mailing list