[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
David CARLIER via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 6 03:50:10 PDT 2024
https://github.com/devnexen updated https://github.com/llvm/llvm-project/pull/83675
>From 39a9b19e266275624e472bd3fbd5fdab542a5c31 Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen at gmail.com>
Date: Sat, 2 Mar 2024 14:56:15 +0000
Subject: [PATCH] [clang][StaticAnalyzer] Adding getentropy to CStringChecker.
since it went way beyond just openbsd, adding basic check for possible
misusage.
---
.../Checkers/CStringChecker.cpp | 49 ++++++++++++++
clang/test/Analysis/bstring.c | 67 +++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 63844563de44f1..25b7e131d84619 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -186,6 +186,8 @@ class CStringChecker : public Checker< eval::Call,
&CStringChecker::evalSprintf},
{{CDM::CLibraryMaybeHardened, {"snprintf"}, std::nullopt, 3},
&CStringChecker::evalSnprintf},
+ {{CDM::CLibraryMaybeHardened, {"getentropy"}, 2},
+ &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -240,6 +242,7 @@ class CStringChecker : public Checker< eval::Call,
void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
bool IsBounded) const;
+ void evalGetentropy(CheckerContext &C, const CallEvent &Call) const;
// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
@@ -2535,6 +2538,52 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C, const CallEvent &Call) const {
+ DestinationArgExpr Buffer = {{Call.getArgExpr(0), 0}};
+ SizeArgExpr Size = {{Call.getArgExpr(1), 1}};
+ ProgramStateRef State = C.getState();
+ SValBuilder &SVB = C.getSValBuilder();
+
+ std::optional<NonLoc> SizeVal = C.getSVal(Size.Expression).getAs<NonLoc>();
+ if (!SizeVal)
+ return;
+
+ std::optional<NonLoc> MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy).getAs<NonLoc>();
+ QualType SizeTy = Size.Expression->getType();
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ auto [StateZeroSize, StateNonZeroSize] =
+ assumeZero(C, State, *SizeVal, SizeTy);
+
+ if (StateZeroSize && !StateNonZeroSize) {
+ State = invalidateDestinationBufferBySize(C, State, Buffer.Expression, Buff, *SizeVal, SizeTy);
+ C.addTransition(State);
+ return;
+ }
+
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+ return;
+
+ State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
+ if (!State)
+ return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ auto [sizeAboveLimit, sizeNotAboveLimit] = State->assume(
+ SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal, *MaxLength, cmpTy)
+ .castAs<DefinedOrUnknownSVal>());
+ if (sizeAboveLimit && !sizeNotAboveLimit) {
+ emitOutOfBoundsBug(C, sizeAboveLimit, Buffer.Expression, "must be smaller than or equal to 256");
+ } else {
+ State = invalidateDestinationBufferBySize(C, sizeNotAboveLimit, Buffer.Expression,
+ Buff,
+ *SizeVal, SizeTy);
+ C.addTransition(State);
+ }
+}
+
//===----------------------------------------------------------------------===//
// The driver method, and other Checker callbacks.
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c
index f015e0b5d9fb7b..1c4810b499b0a9 100644
--- a/clang/test/Analysis/bstring.c
+++ b/clang/test/Analysis/bstring.c
@@ -529,3 +529,70 @@ void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
size_t iAdd = (size_t) addr;
memcpy(((void *) &(s.f)), from, iAdd);
}
+
+//===----------------------------------------------------------------------===//
+// getentropy()
+//===----------------------------------------------------------------------===//
+
+int getentropy(void *d, size_t n);
+
+int getentropy0(void) {
+ char buf[16] = {0};
+
+ int r = getentropy(buf, sizeof(buf)); // no-warning
+ return r;
+}
+
+int getentropy1(void) {
+ char buf[257] = {0};
+
+ int r = getentropy(buf, 256); // no-warning
+ return r;
+}
+
+int getentropy2(void) {
+ char buf[1024] = {0};
+
+ int r = getentropy(buf, sizeof(buf)); // expected-warning{{must be smaller than or equal to 256}}
+ return r;
+}
+
+int getentropy3(void) {
+ char buf[256] = {0};
+
+ int r = getentropy(buf, 0); // no-warning
+ return r;
+}
+
+int getentropy4(size_t arg) {
+ char buf[256] = {0};
+
+ int r = getentropy(buf, arg); // no-warning
+ return r;
+}
+
+int do_something(size_t arg) {
+ char buf[256] = {0};
+ int r = getentropy(buf, arg); // no-warning
+ return r;
+}
+
+int getentropy5(size_t arg) {
+ char buf[257] = {0};
+
+ // split the state and introduce a separate execution path where arg > 256
+ if (arg <= 256)
+ return do_something(arg);
+
+ int r = getentropy(buf, arg); // expected-warning{{must be smaller than or equal to 256}}
+ return r;
+}
+
+int getentropy6(void) {
+ return getentropy(0, 256); // expected-warning{{Null pointer passed as 1st argument to [unix.cstring.NullArg]}}
+}
+
+int getentropy7(void) {
+ char buf[128] = {0};
+ return getentropy(buf, sizeof(buf) * 2); // expected-warning{{'getentropy' will always overflow; destination buffer has size 128, but size argument is 256}}
+}
More information about the cfe-commits
mailing list