zukatsinadze updated this revision to Diff 424493.
zukatsinadze added a comment.
fix clang format
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124244/new/
https://reviews.llvm.org/D124244
Files:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/cert/env30-c.cpp
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -32,6 +32,7 @@
// CHECK-NEXT: core.NullDereference
// CHECK-NEXT: core.StackAddrEscapeBase
// CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.StoreToImmutable
// CHECK-NEXT: core.UndefinedBinaryOperatorResult
// CHECK-NEXT: core.VLASize
// CHECK-NEXT: core.builtin.BuiltinFunctions
Index: clang/test/Analysis/cert/env30-c.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/cert/env30-c.cpp
@@ -0,0 +1,213 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core.StoreToImmutable,alpha.security.cert.env.ModelConstQualifiedReturn \
+// RUN: -analyzer-output=text -verify -Wno-unused %s
+
+#include "../Inputs/system-header-simulator.h"
+char *getenv(const char *name);
+char *setlocale(int category, const char *locale);
+char *strerror(int errnum);
+int setenv(const char *name, const char *value, int overwrite);
+void free(void *memblock);
+void *malloc(size_t size);
+char *strdup(const char *s);
+
+typedef struct {
+} tm;
+char *asctime(const tm *timeptr);
+
+int strcmp(const char *, const char *);
+void non_const_parameter_function(char *e);
+void const_parameter_function(const char *);
+
+void getenv_test1() {
+ char *p = getenv("VAR");
+ // expected-note@-1{{return value of 'getenv' should be treated as const-qualified}}
+ *p = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void getenv_test2() {
+ char *p;
+
+ p = getenv("VAR");
+ non_const_parameter_function(p); // FIXME: Maybe we should warn for these.
+}
+
+void getenv_test3() {
+ char *p;
+ p = getenv("VAR");
+ const_parameter_function(p); // no-warning
+}
+
+void modify(char *p) {
+ *p = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void getenv_test4() {
+ char *p = getenv("VAR");
+ // expected-note@-1{{return value of 'getenv' should be treated as const-qualified}}
+ char *pp = p;
+ modify(pp); // Writing to immutable region within the call.
+ // expected-note@-1{{Calling 'modify'}}
+}
+
+void does_not_modify(char *p) {
+ *p;
+}
+
+void getenv_test5() {
+ char *p = getenv("VAR");
+ does_not_modify(p); // no-warning
+}
+
+void getenv_test6() {
+ static char *array[] = {0};
+
+ if (!array[0]) {
+ // expected-note@-1{{Taking true branch}}
+ array[0] = getenv("TEMPDIR");
+ // expected-note@-1{{return value of 'getenv' should be treated as const-qualified}}
+ }
+
+ *array[0] = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void getenv_test7() {
+ char *p = getenv("VAR");
+ // expected-note@-1{{return value of 'getenv' should be treated as const-qualified}}
+ if (!p)
+ // expected-note@-1{{Assuming 'p' is non-null}}
+ // expected-note@-2{{Taking false branch}}
+ return;
+ p[0] = '\0';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void setlocale_test() {
+ char *p = setlocale(0, "VAR");
+ // expected-note@-1{{return value of 'setlocale' should be treated as const-qualified}}
+ *p = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void strerror_test() {
+ char *p = strerror(0);
+ // expected-note@-1{{return value of 'strerror' should be treated as const-qualified}}
+ *p = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+void asctime_test() {
+ const tm *t;
+ char *p = asctime(t);
+ // expected-note@-1{{return value of 'asctime' should be treated as const-qualified}}
+
+ *p = 'A';
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+}
+
+// Test cases from CERT Rule Page
+
+namespace noncompliant {
+void trstr(char *c_str, char orig, char rep) {
+ while (*c_str != '\0') {
+ // expected-note@-1{{Assuming the condition is true}}
+ // expected-note@-2{{Loop condition is true. Entering loop body}}
+ if (*c_str == orig) {
+ // expected-note@-1{{Assuming the condition is true}}
+ // expected-note@-2{{Taking true branch}}
+ *c_str = rep;
+ // expected-warning@-1{{modifying immutable memory [core.StoreToImmutable]}}
+ // expected-note@-2{{modifying immutable memory}}
+ }
+ ++c_str;
+ }
+}
+
+void func(void) {
+ char *env = getenv("TEST_ENV");
+ // expected-note@-1{{return value of 'getenv' should be treated as const-qualified}}
+ if (env == NULL) {
+ // expected-note@-1{{Assuming 'env' is not equal to NULL}}
+ // expected-note@-2{{Taking false branch}}
+ return;
+ }
+ trstr(env, '"', '_'); // Writing to immutable region within the call.
+ // expected-note@-1{{Calling 'trstr'}}
+}
+
+} // namespace noncompliant
+
+namespace compliant1 {
+void trstr(char *c_str, char orig, char rep) {
+ while (*c_str != '\0') {
+ if (*c_str == orig) {
+ *c_str = rep; // no-warning
+ }
+ ++c_str;
+ }
+}
+
+void func(void) {
+ const char *env;
+ char *copy_of_env;
+
+ env = getenv("TEST_ENV");
+ if (env == NULL) {
+ return;
+ }
+
+ copy_of_env = (char *)malloc(strlen(env) + 1);
+ if (copy_of_env == NULL) {
+ return;
+ }
+
+ strcpy(copy_of_env, env);
+ trstr(copy_of_env, '"', '_');
+ /* ... */
+ free(copy_of_env);
+}
+} // namespace compliant1
+
+namespace compliant2 {
+void trstr(char *c_str, char orig, char rep) {
+ while (*c_str != '\0') {
+ if (*c_str == orig) {
+ *c_str = rep; // no-warning
+ }
+ ++c_str;
+ }
+}
+
+void func(void) {
+ const char *env;
+ char *copy_of_env;
+
+ env = getenv("TEST_ENV");
+ if (env == NULL) {
+ return;
+ }
+
+ copy_of_env = strdup(env);
+ if (copy_of_env == NULL) {
+ return;
+ }
+
+ trstr(copy_of_env, '"', '_');
+
+ if (setenv("TEST_ENV", copy_of_env, 1) != 0) {
+ return;
+ }
+ /* ... */
+ free(copy_of_env);
+}
+} // namespace compliant2
Index: clang/test/Analysis/analyzer-enabled-checkers.c
===================================================================
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -21,6 +21,7 @@
// CHECK-NEXT: core.NullDereference
// CHECK-NEXT: core.StackAddrEscapeBase
// CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.StoreToImmutable
// CHECK-NEXT: core.UndefinedBinaryOperatorResult
// CHECK-NEXT: core.VLASize
// CHECK-NEXT: core.builtin.BuiltinFunctions
Index: clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp
@@ -0,0 +1,92 @@
+//== ModelConstQualifiedReturnChecker.cpp ----------------------- -*- C++
+//-*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines ModelConstQualifiedReturnChecker
+// Return values of certain functions should be treated as const-qualified
+//
+// CERT SEI Rule ENV30-C
+// For more information see: https://wiki.sei.cmu.edu/confluence/x/79UxBQ
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class ModelConstQualifiedReturnChecker : public Checker<eval::Call> {
+private:
+ void evalConstQualifiedReturnCall(const CallEvent &Call,
+ CheckerContext &C) const;
+
+ // SEI CERT ENV30-C
+ const CallDescriptionSet ConstQualifiedReturnFunctions = {
+ {"getenv", 1}, {"setlocale", 2}, {"strerror", 1},
+ {"localeconv", 0}, {"asctime", 1},
+ };
+
+public:
+ bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+};
+} // namespace
+
+void ModelConstQualifiedReturnChecker::evalConstQualifiedReturnCall(
+ const CallEvent &Call, CheckerContext &C) const {
+ const LocationContext *LCtx = C.getLocationContext();
+ SValBuilder &SVB = C.getSValBuilder();
+ MemRegionManager &RegMgr = SVB.getRegionManager();
+
+ // Function call will return a pointer to the new symbolic region.
+ SymbolRef FreshSym = SVB.getSymbolManager().conjureSymbol(
+ Call.getOriginExpr(), LCtx, C.blockCount());
+ const SymbolicRegion *SymReg = RegMgr.getSymbolicRegion(
+ FreshSym,
+ RegMgr.getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind));
+
+ ProgramStateRef State = C.getState();
+ State =
+ State->BindExpr(Call.getOriginExpr(), LCtx, loc::MemRegionVal{SymReg});
+
+ StringRef FunName = Call.getCalleeIdentifier()->getName();
+ const NoteTag *Note = C.getNoteTag(
+ [SymReg, FunName](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ if (BR.getBugType().getCheckerName() != "core.StoreToImmutable" ||
+ !BR.isInteresting(SymReg))
+ return;
+ Out << "return value of '" << FunName
+ << "' should be treated as const-qualified";
+ });
+
+ C.addTransition(State, Note);
+}
+
+bool ModelConstQualifiedReturnChecker::evalCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (!ConstQualifiedReturnFunctions.contains(Call))
+ return false;
+
+ evalConstQualifiedReturnCall(Call, C);
+ return true;
+}
+
+void ento::registerModelConstQualifiedReturnChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<ModelConstQualifiedReturnChecker>();
+}
+
+bool ento::shouldRegisterModelConstQualifiedReturnChecker(
+ const CheckerManager &) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
@@ -0,0 +1,62 @@
+//== StoreToImmutableChecker.cpp ------------------------------- -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines StoreToImmutableChecker which warns binds on immutable
+// memory.
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/Type.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class StoreToImmutableChecker : public Checker<check::Bind> {
+private:
+ BugType ImmutableMemoryBind{this,
+ "Immutable memory should not be overwritten",
+ categories::MemoryError};
+
+public:
+ void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const;
+};
+} // namespace
+
+// Check if region that should be immutable is being modified
+void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
+ CheckerContext &C) const {
+ const MemRegion *R = Loc.getAsRegion();
+ if (!R)
+ return;
+
+ if (!isa<GlobalImmutableSpaceRegion>(R->getMemorySpace()))
+ return;
+
+ ExplodedNode *ErrorNode = C.generateErrorNode();
+ if (!ErrorNode)
+ return;
+
+ auto Report = std::make_unique<PathSensitiveBugReport>(
+ ImmutableMemoryBind, "modifying immutable memory", ErrorNode);
+ Report->markInteresting(R);
+ C.emitReport(std::move(Report));
+}
+
+void ento::registerStoreToImmutableChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<StoreToImmutableChecker>();
+}
+
+bool ento::shouldRegisterStoreToImmutableChecker(const CheckerManager &) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -66,6 +66,7 @@
MismatchedIteratorChecker.cpp
MmapWriteExecChecker.cpp
MIGChecker.cpp
+ cert/ModelConstQualifiedReturnChecker.cpp
MoveChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
@@ -106,6 +107,7 @@
StackAddrEscapeChecker.cpp
StdLibraryFunctionsChecker.cpp
STLAlgorithmModeling.cpp
+ StoreToImmutableChecker.cpp
StreamChecker.cpp
StringChecker.cpp
Taint.cpp
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -765,7 +765,8 @@
s->getType()->isReferenceType() ||
s->getType()->isBlockPointerType());
assert(isa<UnknownSpaceRegion>(sreg) || isa<HeapSpaceRegion>(sreg) ||
- isa<GlobalSystemSpaceRegion>(sreg));
+ isa<GlobalSystemSpaceRegion>(sreg) ||
+ isa<GlobalImmutableSpaceRegion>(sreg));
}
public:
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -220,6 +220,10 @@
Dependencies<[StackAddrEscapeBase]>,
Documentation<HasDocumentation>;
+def StoreToImmutableChecker : Checker<"StoreToImmutable">,
+ HelpText<"Check for writes to immutable memory.">,
+ Documentation<HasDocumentation>;
+
def DynamicTypePropagation : Checker<"DynamicTypePropagation">,
HelpText<"Generate dynamic type information">,
Documentation<NotDocumented>,
@@ -972,6 +976,12 @@
HelpText<"Finds usages of possibly invalidated pointers">,
Documentation<HasDocumentation>;
+ def ModelConstQualifiedReturnChecker : Checker<"ModelConstQualifiedReturn">,
+ HelpText<"Model return values of some functions as const-qualified. "
+ "StoreToImmutable checker would warn on their modification.">,
+ Documentation<HasDocumentation>;
+
+
} // end "alpha.cert.env"
let ParentPackage = SecurityAlpha in {
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -132,6 +132,21 @@
x = &y; // warn
}
+.. _core-StoreToImmutable:
+
+core.StoreToImmutable (C, C++)
+"""""""""""""""""""""""""""""""""""""""
+Check for writes to immutable memory.
+
+.. code-block:: cpp
+
+ void write_to_envvar() {
+ char *p = getenv("VAR");
+ if (!p)
+ return;
+ p[0] = '\0'; // warn
+ }
+
.. _core-UndefinedBinaryOperatorResult:
@@ -2308,6 +2323,33 @@
// dereferencing invalid pointer
}
+.. _alpha-security-cert-env-ModelConstQualifiedReturn:
+
+alpha.security.cert.env.ModelConstQualifiedReturn
+"""""""""""""""""""""""""""""""""""""""""""""""""
+
+Corresponds to SEI CERT Rules ENV30-C.
+
+Some functions return a pointer to an object that cannot be
+modified without causing undefined behavior. These functions
+include getenv(), setlocale(), localeconv(), asctime(), and
+strerror(). In such cases, the function call results must be
+treated as being const-qualified.
+
+Checker models return values of these functions as const
+qualified. Their modification is checked in StoreToImmutable
+core checker.
+
+.. code-block:: c
+
+ void writing_to_envvar() {
+ char *p = getenv("VAR"); // note: getenv return value
+ // should be treated as const
+ if (!p)
+ return;
+ p[0] = '\0'; // warn
+ }
+
alpha.security.taint
^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
