[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
David CARLIER via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 8 07:12:11 PST 2024
https://github.com/devnexen updated https://github.com/llvm/llvm-project/pull/83675
>From 1b2fec2c9a41be4ad216d7032189f561eed3f751 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 1/3] [clang][StaticAnalyzer] Adding getentropy to
CStringChecker.
since it went way beyond just openbsd, adding basic check for possible
misusage.
---
.../Checkers/CStringChecker.cpp | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 59be236ca1c769..cea99fad3e8436 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -165,6 +165,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDM::CLibrary, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDM::CLibrary, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDM::CLibrary, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
+ {{CDM::CLibrary, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -219,6 +220,7 @@ class CStringChecker : public Checker< eval::Call,
void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
bool IsBounded, bool IsBuiltin) const;
+ void evalGetentropy(CheckerContext &C, const CallEvent &Call) const;
// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
@@ -2515,6 +2517,47 @@ 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();
+ constexpr int BufferMaxSize = 256;
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+ return;
+
+ State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
+ if (!State)
+ return;
+
+ auto SizeLoc = SizeVal.getAs<nonloc::ConcreteInt>();
+ auto size = SizeLoc->getValue().getExtValue();
+
+ if (size > BufferMaxSize) {
+ ErrorMessage Message;
+ llvm::raw_svector_ostream Os(Message);
+ Os << " destination buffer size is greater than " << BufferMaxSize;
+ emitOutOfBoundsBug(C, StateNonZeroSize, Buffer.Expression, Message);
+ return;
+ }
+
+ State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
+ C.getSVal(Buffer.Expression),
+ SizeVal, SizeTy);
+
+ C.addTransition(State);
+}
+
//===----------------------------------------------------------------------===//
// The driver method, and other Checker callbacks.
//===----------------------------------------------------------------------===//
>From 4c626fa147aade7725e04dc633b53aefcd1347b0 Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen at gmail.com>
Date: Wed, 6 Mar 2024 17:38:25 +0000
Subject: [PATCH 2/3] few fixes and tests additions
---
.../Checkers/CStringChecker.cpp | 51 +++++++++++--------
clang/test/Analysis/bstring.c | 39 ++++++++++++++
2 files changed, 70 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index cea99fad3e8436..4d0492bcaf159e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -165,7 +165,8 @@ class CStringChecker : public Checker< eval::Call,
{{CDM::CLibrary, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDM::CLibrary, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDM::CLibrary, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
- {{CDM::CLibrary, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
+ {{CDM::CLibrary, {"getentropy"}, 2},
+ std::bind(&CStringChecker::evalGetentropy, _1, _2, _3, CK_Regular)},
};
// These require a bit of special handling.
@@ -220,7 +221,7 @@ class CStringChecker : public Checker< eval::Call,
void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
bool IsBounded, bool IsBuiltin) const;
- void evalGetentropy(CheckerContext &C, const CallEvent &Call) const;
+ void evalGetentropy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
@@ -2518,11 +2519,13 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
}
void CStringChecker::evalGetentropy(CheckerContext &C,
- const CallEvent &Call) const {
+ const CallEvent &Call, CharKind CK) const {
DestinationArgExpr Buffer = {{Call.getArgExpr(0), 0}};
SizeArgExpr Size = {{Call.getArgExpr(1), 1}};
ProgramStateRef State = C.getState();
- constexpr int BufferMaxSize = 256;
+ const LocationContext *LCtx = C.getLocationContext();
+ SValBuilder &Builder = C.getSValBuilder();
+ SVal MaxLength = Builder.makeIntVal(256, C.getASTContext().IntTy);
SVal SizeVal = C.getSVal(Size.Expression);
QualType SizeTy = Size.Expression->getType();
@@ -2531,31 +2534,39 @@ void CStringChecker::evalGetentropy(CheckerContext &C,
std::tie(StateZeroSize, StateNonZeroSize) =
assumeZero(C, State, SizeVal, SizeTy);
- SVal Buff = C.getSVal(Buffer.Expression);
- State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
- if (!State)
+ if (StateZeroSize) {
+ StateZeroSize = State->BindExpr(Call.getOriginExpr(), LCtx,
+ Builder.makeZeroVal(C.getASTContext().IntTy));
+ C.addTransition(StateZeroSize);
return;
+ }
- State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
if (!State)
return;
- auto SizeLoc = SizeVal.getAs<nonloc::ConcreteInt>();
- auto size = SizeLoc->getValue().getExtValue();
-
- if (size > BufferMaxSize) {
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef bufferTooLong, bufferNotTooLong;
+ std::tie(bufferTooLong, bufferNotTooLong) = State->assume(
+ Builder
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs<NonLoc>(), *MaxLength.getAs<NonLoc>(), cmpTy)
+ .castAs<DefinedOrUnknownSVal>());
+ if (bufferTooLong) {
ErrorMessage Message;
llvm::raw_svector_ostream Os(Message);
- Os << " destination buffer size is greater than " << BufferMaxSize;
- emitOutOfBoundsBug(C, StateNonZeroSize, Buffer.Expression, Message);
- return;
- }
+ Os << "size is greater than 256";
+ emitOutOfBoundsBug(C, bufferTooLong, Buffer.Expression, Message);
+ } else {
+ State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write, CK);
+ if (!State)
+ return;
- State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
- C.getSVal(Buffer.Expression),
+ State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
+ Buff,
SizeVal, SizeTy);
-
- C.addTransition(State);
+ C.addTransition(State);
+ }
}
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c
index f015e0b5d9fb7b..0696e98f8417dd 100644
--- a/clang/test/Analysis/bstring.c
+++ b/clang/test/Analysis/bstring.c
@@ -529,3 +529,42 @@ void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
size_t iAdd = (size_t) addr;
memcpy(((void *) &(s.f)), from, iAdd);
}
+
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__OpenBSD__)
+/* present in both glibc 2.25 and musl 1.1.20 */
+
+//===----------------------------------------------------------------------===
+// 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{{size is greater than 256}}
+ return r;
+}
+
+int getentropy3(void) {
+ char buf[256] = {0};
+
+ int r = getentropy(buf, 0); // no-wwarning
+ return r;
+}
+
+#endif
>From f10b99a978b6833e24eb098898d71bc54217b67d Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen at gmail.com>
Date: Fri, 8 Mar 2024 15:11:59 +0000
Subject: [PATCH 3/3] further changes from feedback
---
.../Checkers/CStringChecker.cpp | 31 ++++++++-----------
clang/test/Analysis/bstring.c | 11 ++-----
2 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 4d0492bcaf159e..45c1d51c71d2c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -165,8 +165,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDM::CLibrary, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDM::CLibrary, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDM::CLibrary, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
- {{CDM::CLibrary, {"getentropy"}, 2},
- std::bind(&CStringChecker::evalGetentropy, _1, _2, _3, CK_Regular)},
+ {{CDM::CLibrary, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -221,7 +220,7 @@ class CStringChecker : public Checker< eval::Call,
void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
bool IsBounded, bool IsBuiltin) const;
- void evalGetentropy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+ void evalGetentropy(CheckerContext &C, const CallEvent &Call) const;
// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
@@ -2518,14 +2517,12 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
C.addTransition(State);
}
-void CStringChecker::evalGetentropy(CheckerContext &C,
- const CallEvent &Call, CharKind CK) const {
+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();
- const LocationContext *LCtx = C.getLocationContext();
- SValBuilder &Builder = C.getSValBuilder();
- SVal MaxLength = Builder.makeIntVal(256, C.getASTContext().IntTy);
+ SValBuilder &SVB = C.getSValBuilder();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
SVal SizeVal = C.getSVal(Size.Expression);
QualType SizeTy = Size.Expression->getType();
@@ -2535,8 +2532,8 @@ void CStringChecker::evalGetentropy(CheckerContext &C,
assumeZero(C, State, SizeVal, SizeTy);
if (StateZeroSize) {
- StateZeroSize = State->BindExpr(Call.getOriginExpr(), LCtx,
- Builder.makeZeroVal(C.getASTContext().IntTy));
+ StateZeroSize = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
C.addTransition(StateZeroSize);
return;
}
@@ -2547,18 +2544,16 @@ void CStringChecker::evalGetentropy(CheckerContext &C,
return;
QualType cmpTy = C.getSValBuilder().getConditionType();
- ProgramStateRef bufferTooLong, bufferNotTooLong;
- std::tie(bufferTooLong, bufferNotTooLong) = State->assume(
- Builder
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+ SVB
.evalBinOpNN(State, BO_GT, *SizeVal.getAs<NonLoc>(), *MaxLength.getAs<NonLoc>(), cmpTy)
.castAs<DefinedOrUnknownSVal>());
- if (bufferTooLong) {
+ if (sizeAboveLimit) {
ErrorMessage Message;
- llvm::raw_svector_ostream Os(Message);
- Os << "size is greater than 256";
- emitOutOfBoundsBug(C, bufferTooLong, Buffer.Expression, Message);
+ emitOutOfBoundsBug(C, sizeAboveLimit, Buffer.Expression, "must be smaller than or equal to 256");
} else {
- State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write, CK);
+ State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
if (!State)
return;
diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c
index 0696e98f8417dd..10ceb27690909f 100644
--- a/clang/test/Analysis/bstring.c
+++ b/clang/test/Analysis/bstring.c
@@ -530,12 +530,9 @@ void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
memcpy(((void *) &(s.f)), from, iAdd);
}
-#if defined(__linux__) || defined(__FreeBSD__) || defined(__OpenBSD__)
-/* present in both glibc 2.25 and musl 1.1.20 */
-
-//===----------------------------------------------------------------------===
+//===----------------------------------------------------------------------===//
// getentropy()
-//===----------------------------------------------------------------------===
+//===----------------------------------------------------------------------===//
int getentropy(void *d, size_t n);
@@ -556,7 +553,7 @@ int getentropy1(void) {
int getentropy2(void) {
char buf[1024] = {0};
- int r = getentropy(buf, sizeof(buf)); // expected-warning{{size is greater than 256}}
+ int r = getentropy(buf, sizeof(buf)); // expected-warning{{must be smaller than or equal to 256}}
return r;
}
@@ -566,5 +563,3 @@ int getentropy3(void) {
int r = getentropy(buf, 0); // no-wwarning
return r;
}
-
-#endif
More information about the cfe-commits
mailing list