[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