[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
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
Date: Sat, 2 Mar 2024 14:56:15 +
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
@@ -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 SizeVal = C.getSVal(Size.Expression).getAs();
+ if (!SizeVal)
+return;
+
+ std::optional MaxLength = SVB.makeIntVal(256,
C.getASTContext().IntTy).getAs();
+ 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());
+ 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}
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (sizeAboveLimit) {
+ErrorMessage Message;
+emitOutOfBoundsBug(C, sizeAboveLimit, Buffer.Expression, "must be smaller
than or equal to 256");
+ } else {
+State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
+if (!State)
+ return;
+
+State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
+Buff,
+SizeVal, SizeTy);
+C.addTransition(State);
balazske wrote:
This branch separation and setting of `errno` can be done with
`StdLibraryFunctionsChecker`. If in this checker the return value is not
important it is enough to assume it is in the correct range. The other checker
runs in `postCall` and can change the state further.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/devnexen updated
https://github.com/llvm/llvm-project/pull/83675
>From 010c0c2acddbe36a84382284835e94bffe94b040 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Sat, 2 Mar 2024 14:56:15 +
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
@@ -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();
+ 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 2866da018b137f2c099f733920a1e15b7e41d289 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Wed, 6 Mar 2024 17:38:25 +
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 me
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
github-actions[bot] wrote: :white_check_mark: With the latest revision this PR passed the Python code formatter. https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/devnexen updated
https://github.com/llvm/llvm-project/pull/83675
>From 5e99ec4cbc47b513c54f2579529aed611cd8b847 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Sat, 2 Mar 2024 14:56:15 +
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 59be236ca1c7695..cea99fad3e84367 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
@@ -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();
+ 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 7c9e5463947ceb7fa17bfeab7df243411907904b Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Wed, 6 Mar 2024 17:38:25 +
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 cea99fad3e84367..4d0492bcaf159e4 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;
// Utilit
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/NagyDonat requested changes to this pull request. I'll try to take over this review process and help with finalizing this commit. I also added @balazske who's also familiar with this area. Unfortunately currently there are significant problems in the state/assumption manipulation logic: it looks as if you thought that functions like `assumeZero` and `assume` always returned one nullpointer and one non-null state ref (in either order) -- while they have a third possibility where both returned references are non-null. https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (sizeAboveLimit) {
NagyDonat wrote:
```suggestion
if (sizeAboveLimit && !sizeNotAboveLimit) {
```
In ambiguous situations (when both situations may be possible) the checkers
must be lenient -- they should report errors only when they're certain that the
error will happen. (Otherwise we'd get an overwhelming amount of false
positives from the code parts where the analyzer doesn't know the values of the
variables.)
The only exception is that if a value is marked as "tainted" (attacker
controlled, see the GenericTaint checker which can mark values as tainted),
then we assume the worst about it (if the situation is ambiguous and a tainted
variable is involved, then we report an error). Some checkers are "taint aware"
and incorporate this check into their logic, but it's perfectly fine if a
checker is not (yet) taint aware, so you shouldn't bother with this (in this
commit).
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (sizeAboveLimit) {
+ErrorMessage Message;
NagyDonat wrote:
```suggestion
```
This seems to be unused.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -529,3 +529,37 @@ 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-wwarning
+ return r;
+}
NagyDonat wrote:
Add testcases like
```
int getentropy4(size_t arg) {
char buf[257] = {0};
int r = getentropy(buf, arg); // no-warning
return r;
}
void do_something();
int getentropy5(size_t arg) {
char buf[257] = {0};
// split the state and introduce a separate execution path where arg > 256
if (arg <= 256)
do_something();
int r = getentropy(buf, arg); // expected-warning{{must be smaller than or
equal to 256}}
return r;
}
```
and also create a few testcases where `getentropy` fails because the buffer is
a nullpointer or the specified size is larger than the buffer size.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -529,3 +529,37 @@ 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-wwarning
NagyDonat wrote:
```suggestion
int r = getentropy(buf, 0); // no-warning
```
Just a typo. (By the way, "no-warning" is just a comment, it's not significant
for the test engine. The tests will fail if when any unexpected warning
appears, but it's customary to write no-warning after the statements that are
the "central" parts of a testcase but should not produce a warning.)
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
NagyDonat wrote:
```suggestion
auto [sizeAboveLimit, sizeNotAboveLimit] = State->assume(
```
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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);
NagyDonat wrote:
Yes, if the current code encounters a situation when the size may be zero _and_
may be nonzero (so `assumeZero` splits the state and returns two non-null state
references), then it will act as if the size argument was _known to be_ zero
and transitions to a state where the return value is _known to be_ zero.
You need to explicitly handle this ambiguous case somehow:
(1) either you do a state split and create two separate transitions to the two
possible branches (and create note tags that e.g. say "assuming the 'size'
argument is zero" on the branch where you assume that)
(2) or, alternatively, you keep a single transition and try to ensure that the
potentially affected values are marked as unknown (e.g. don't bind a value to
the call expression and probably you should invalidate the buffer contents).
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (sizeAboveLimit) {
+ErrorMessage Message;
+emitOutOfBoundsBug(C, sizeAboveLimit, Buffer.Expression, "must be smaller
than or equal to 256");
+ } else {
+State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
+if (!State)
+ return;
+
+State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
+Buff,
+SizeVal, SizeTy);
+C.addTransition(State);
NagyDonat wrote:
According to the man page, the return value of this function is either 0
(success) or -1 (failure), so you should probably bind a conjured symbolic
value to the call expression and assume that it's in the range [-1, 0].
Moreover, it would be good to handle the success and failure as separate
branches (modeling the fact that e.g. `errno` is set on a failure), but that
could be left for a follow-up commit.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
NagyDonat wrote:
```suggestion
auto [StateZeroSize, StateNonZeroSize] =
```
We have C++17, we can use structured bindings instead of `std::tie`.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (sizeAboveLimit) {
+ErrorMessage Message;
+emitOutOfBoundsBug(C, sizeAboveLimit, Buffer.Expression, "must be smaller
than or equal to 256");
+ } else {
+State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
NagyDonat wrote:
```suggestion
State = CheckBufferAccess(C, sizeNotAboveLimit, Buffer, Size,
AccessKind::write);
```
As I said before, try to use the most recent state value.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
+ SVB.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef sizeAboveLimit, sizeNotAboveLimit;
+ std::tie(sizeAboveLimit, sizeNotAboveLimit) = State->assume(
+SVB
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
NagyDonat wrote:
Declare `SizeVal` and `MaxLength` as `NonLoc` instead of doing this immediately
dereferenced `getAs()`. In the case of `SizeVal` you should do an early return
in the unlikely case when the value is not a `NonLoc`.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2517,53 @@ 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();
+ SVal MaxLength = SVB.makeIntVal(256, C.getASTContext().IntTy);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
NagyDonat wrote:
```suggestion
StateZeroSize = StateZeroSize->BindExpr(Call.getOriginExpr(),
C.getLocationContext(),
```
For the sake of consistency always avoid using "stale" state values, because
this leads to loss of information and inconsistencies.
The only situation where this is not important is the case when you perform a
dual assumption (an assume call that returns two state references, e.g. the
`assumeZero` above this) _and_ you checked that one of the two state references
is NULL. In that case the other returned state reference will be practically
equivalent to the state before the assumption (but even then there are some
little arcane details that may differ).
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
steakhal wrote: I'm sorry, but I'm out of bandwidth. Maybe someone else can step up for the review. https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
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
Date: Sat, 2 Mar 2024 14:56:15 +
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
@@ -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();
+ 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
Date: Wed, 6 Mar 2024 17:38:25 +
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 me
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2518,57 @@ void CStringChecker::evalSprintfCommon(CheckerContext
&C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C,
+const CallEvent &Call, CharKind CK) const {
+ DestinationArgExpr Buffer = {{Call.getArgExpr(0), 0}};
+ SizeArgExpr Size = {{Call.getArgExpr(1), 1}};
+ ProgramStateRef State = C.getState();
+ const LocationContext *LCtx = C.getLocationContext();
steakhal wrote:
This is only used once. I think you could directly use there
`C.getLocationContext()`.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -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()
+//===--===
steakhal wrote:
If you prefer to keep this header, make it 81 chars wide.
```suggestion
//===--===//
// getentropy()
//===--===//
```
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2518,57 @@ void CStringChecker::evalSprintfCommon(CheckerContext
&C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C,
+const CallEvent &Call, CharKind CK) 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();
steakhal wrote:
```suggestion
SValBuilder &SVB = C.getSValBuilder();
```
We usually call this `SVB`.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -219,6 +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, CharKind CK) const; steakhal wrote: ```suggestion void evalGetentropy(CheckerContext &C, const CallEvent &Call) const; ``` https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2518,57 @@ void CStringChecker::evalSprintfCommon(CheckerContext
&C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C,
+const CallEvent &Call, CharKind CK) 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);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(), LCtx,
+ Builder.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef bufferTooLong, bufferNotTooLong;
+ std::tie(bufferTooLong, bufferNotTooLong) = State->assume(
+Builder
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (bufferTooLong) {
steakhal wrote:
We would take this branch (and report `size is greater than 256`), if we could
not prove that it must be smaller then or equal to 256. We usually report
diagnostics if the error condition must be violated in a given execution path.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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();
+ 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;
+ }
steakhal wrote:
I see, now you comment makes sense!
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2518,57 @@ void CStringChecker::evalSprintfCommon(CheckerContext
&C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C,
+const CallEvent &Call, CharKind CK) 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);
+
+ SVal SizeVal = C.getSVal(Size.Expression);
+ QualType SizeTy = Size.Expression->getType();
+
+ ProgramStateRef StateZeroSize, StateNonZeroSize;
+ std::tie(StateZeroSize, StateNonZeroSize) =
+ assumeZero(C, State, SizeVal, SizeTy);
+
+ if (StateZeroSize) {
+StateZeroSize = State->BindExpr(Call.getOriginExpr(), LCtx,
+ Builder.makeZeroVal(C.getASTContext().IntTy));
+C.addTransition(StateZeroSize);
+return;
+ }
+
+ SVal Buff = C.getSVal(Buffer.Expression);
+ State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
+ if (!State)
+return;
+
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+ ProgramStateRef bufferTooLong, bufferNotTooLong;
+ std::tie(bufferTooLong, bufferNotTooLong) = State->assume(
+Builder
+ .evalBinOpNN(State, BO_GT, *SizeVal.getAs(),
*MaxLength.getAs(), cmpTy)
+ .castAs());
+ if (bufferTooLong) {
+ErrorMessage Message;
+llvm::raw_svector_ostream Os(Message);
+Os << "size is greater than 256";
+emitOutOfBoundsBug(C, bufferTooLong, Buffer.Expression, Message);
steakhal wrote:
```suggestion
emitOutOfBoundsBug(C, bufferTooLong, Buffer.Expression, "size is greater
than 256");
```
But actually, I don't think an "out of bounds" error is appropriate here; and
the provided message could be rephrased to "the 'length' argument to
'getentropy' must be smaller than or equal to 256". This hints the user how to
fix this. @haoNoQ WDYT of reporting this as an out-of-bounds access?
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2515,6 +2518,57 @@ void CStringChecker::evalSprintfCommon(CheckerContext
&C, const CallEvent &Call,
C.addTransition(State);
}
+void CStringChecker::evalGetentropy(CheckerContext &C,
+const CallEvent &Call, CharKind CK) const {
steakhal wrote:
```suggestion
void CStringChecker::evalGetentropy(CheckerContext &C, const CallEvent &Call)
const {
```
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -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 */
steakhal wrote:
We add the declaration in the test, so I don't think we should conditionally
include this test.
```suggestion
```
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -165,6 +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},
+ std::bind(&CStringChecker::evalGetentropy, _1, _2, _3, CK_Regular)},
steakhal wrote:
```suggestion
{{CDM::CLibrary, {"getentropy"}, 2}, CStringChecker::evalGetentropy},
```
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/steakhal requested changes to this pull request. https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
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
Date: Sat, 2 Mar 2024 14:56:15 +
Subject: [PATCH 1/2] [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
@@ -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();
+ 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
Date: Wed, 6 Mar 2024 17:38:25 +
Subject: [PATCH 2/2] 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 me
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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();
+ 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;
+ }
steakhal wrote:
Inside `checkBufferAccess` there should be a part where the `size-1` index is
checked for buffers; thus it should cover the case you mentioned.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
balazske wrote: Currently it looks OK to add `getentropy` to this checker because it is a string related function in a way. Otherwise it looks like that many of the checks (for buffer access, and buffer invalidations) that are implemented in `CStringChecker` could be moved to `StdLibraryFunctionsChecker` (but this can be more work). https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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();
+ 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;
+ }
steakhal wrote:
```suggestion
```
This hunk seems to be unnecessary as `CheckBufferAccess` supposed to check both
ends of the buffer.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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();
+ 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);
steakhal wrote:
I think you did not evaluate the return value for this eval call. You must bind
a value as the call expression in eval call.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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);
steakhal wrote:
I think you forgot to bail out if `StateZeroSize && StateNonZeroSize`.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
@@ -2516,6 +2518,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);
steakhal wrote:
`StateNonZeroSize` might be null here; see my previous comment.
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/steakhal requested changes to this pull request. Thanks for the PR! At first I was hesitant if this checker is the right place for this API. But actually, it should be fine to have it here. Maybe the stdlibraryfunctionschecker would be a better place in long term, but I don't think that has DSL for buffer accesses, like we have here. Maybe @balazske has some opinion on this. Anyways, I'd like to see tests for about any aspects of this API. - Under what conditions it writes to the buffer (aka. length > 0). - When can the checker issue a diagnostic (null buffer, small buffer) - What if `length` is symbolic and constrained to be really large (e.g. `length > 300`) - What if the `length` and the `buffer` is symbolic (unconstrained), did we infer that `length <= 256` after the call? - etc, could be more cases, but I only named what came in my mind. https://github.com/llvm/llvm-project/pull/83675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/devnexen updated
https://github.com/llvm/llvm-project/pull/83675
>From 685c7e56c1ce8d2e11c0f9a97f6c4d24f63a05b8 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Sat, 2 Mar 2024 14:56:15 +
Subject: [PATCH] [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 b7b64c3da4f6c8..5b4c3912f13006 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -166,6 +166,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDF_MaybeBuiltin, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDF_MaybeBuiltin, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
+ {{CDF_MaybeBuiltin, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -220,6 +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;
// Utility methods
std::pair
@@ -2516,6 +2518,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();
+ 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.
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff ca827d53c5524409dcca5ade3949b25f38a60fef
f9e571bfa3e64d9fb54e965f3c363aef40fa3b80 --
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b6b0878459..5b4c3912f1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2518,7 +2518,8 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C,
const CallEvent &Call,
C.addTransition(State);
}
-void CStringChecker::evalGetentropy(CheckerContext &C, const CallEvent &Call)
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();
@@ -2529,7 +2530,7 @@ void CStringChecker::evalGetentropy(CheckerContext &C,
const CallEvent &Call) co
ProgramStateRef StateZeroSize, StateNonZeroSize;
std::tie(StateZeroSize, StateNonZeroSize) =
-assumeZero(C, State, SizeVal, SizeTy);
+ assumeZero(C, State, SizeVal, SizeTy);
SVal Buff = C.getSVal(Buffer.Expression);
State = checkNonNull(C, StateNonZeroSize, Buffer, Buff);
@@ -2551,9 +2552,9 @@ void CStringChecker::evalGetentropy(CheckerContext &C,
const CallEvent &Call) co
return;
}
- State = invalidateDestinationBufferBySize(
- C, State, Buffer.Expression, C.getSVal(Buffer.Expression), SizeVal,
- SizeTy);
+ State = invalidateDestinationBufferBySize(C, State, Buffer.Expression,
+C.getSVal(Buffer.Expression),
+SizeVal, SizeTy);
C.addTransition(State);
}
``
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: David CARLIER (devnexen)
Changes
since it went way beyond just openbsd, adding basic check for possible misusage.
---
Full diff: https://github.com/llvm/llvm-project/pull/83675.diff
1 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (+42)
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b7b64c3da4f6c8..b6b0878459f0c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -166,6 +166,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDF_MaybeBuiltin, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDF_MaybeBuiltin, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
+ {{CDF_MaybeBuiltin, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -220,6 +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;
// Utility methods
std::pair
@@ -2516,6 +2518,46 @@ 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();
+ 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.
//===--===//
``
https://github.com/llvm/llvm-project/pull/83675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Adding getentropy to CStringChecker. (PR #83675)
https://github.com/devnexen created
https://github.com/llvm/llvm-project/pull/83675
since it went way beyond just openbsd, adding basic check for possible misusage.
>From f9e571bfa3e64d9fb54e965f3c363aef40fa3b80 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Sat, 2 Mar 2024 14:56:15 +
Subject: [PATCH] [clang][StaticAnalyzer] Adding getentropy to CStringChecker.
since it went way beyond just openbsd, adding basic check for possible
misusage.
---
.../Checkers/CStringChecker.cpp | 42 +++
1 file changed, 42 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b7b64c3da4f6c8..b6b0878459f0c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -166,6 +166,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDF_MaybeBuiltin, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDF_MaybeBuiltin, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
+ {{CDF_MaybeBuiltin, {"getentropy"}, 2}, &CStringChecker::evalGetentropy},
};
// These require a bit of special handling.
@@ -220,6 +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;
// Utility methods
std::pair
@@ -2516,6 +2518,46 @@ 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();
+ 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.
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
