[clang] [analyzer] Fix StreamChecker crash in fread modeling (PR #108393)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 12 06:45:02 PDT 2024


https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/108393

In #93408 https://github.com/llvm/llvm-project/commit/69bc159142c6e4ed168e32a6168392d396f891de
I refined how invalidation is done for `fread`. It can crash, if the "size" or "count" parameters of "fread" is a perfectly constrained negative value. In such cases, when it will try to allocate a SmallVector with a negative size, which will cause a crash.

To mitigate this issue, let's just guard against negative values.

CPP-3247

>From d2e5e284980843867652df499b05169cc6058c36 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Thu, 12 Sep 2024 15:20:19 +0200
Subject: [PATCH] [analyzer] Fix StreamChecker crash in fread modeling

In #93408
https://github.com/llvm/llvm-project/commit/69bc159142c6e4ed168e32a6168392d396f891de

I refined how invalidation is done for `fread`. It can crash, if the
"size" or "count" parameters of "fread" is a perfectly contrained
negative value. In such cases, when it will try to allocate a
SmallVEctor with a negative size, which will cause a crash.

To mitigate this issue, let's just guard against negative values.

CPP-3247
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp |  2 +-
 clang/test/Analysis/fread.c                   | 30 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 22061373c4b393..8bb7880a3cc283 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1129,7 +1129,7 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
   if (!ElemTy.isNull() && CountVal && Size && StartIndexVal) {
     int64_t NumBytesRead = Size.value() * CountVal.value();
     int64_t ElemSizeInChars = Ctx.getTypeSizeInChars(ElemTy).getQuantity();
-    if (ElemSizeInChars == 0)
+    if (ElemSizeInChars == 0 || NumBytesRead < 0)
       return nullptr;
 
     bool IncompleteLastElement = (NumBytesRead % ElemSizeInChars) != 0;
diff --git a/clang/test/Analysis/fread.c b/clang/test/Analysis/fread.c
index 3f286421fd7a13..d470f0abebe621 100644
--- a/clang/test/Analysis/fread.c
+++ b/clang/test/Analysis/fread.c
@@ -443,3 +443,33 @@ void test_unaligned_start_read(void) {
     fclose(fp);
   }
 }
+
+void no_crash_if_count_is_negative(long s, unsigned char *buffer) {
+  FILE *fp = fopen("path", "r");
+  if (fp) {
+    if (s * s == -1) {
+      fread(buffer, 1, s * s, fp); // no-crash
+    }
+    fclose(fp);
+  }
+}
+
+void no_crash_if_size_is_negative(long s, unsigned char *buffer) {
+  FILE *fp = fopen("path", "r");
+  if (fp) {
+    if (s * s == -1) {
+      fread(buffer, s * s, 1, fp); // no-crash
+    }
+    fclose(fp);
+  }
+}
+
+void no_crash_if_size_and_count_are_negative(long s, unsigned char *buffer) {
+  FILE *fp = fopen("path", "r");
+  if (fp) {
+    if (s * s == -1) {
+      fread(buffer, s * s, s * s, fp); // no-crash
+    }
+    fclose(fp);
+  }
+}



More information about the cfe-commits mailing list