diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 4e8b31869bb56..b2effadacf9f1 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3086,6 +3086,23 @@ Either the comparison is useless or there is division by zero. if (x == 0) { } // warn } +.. _alpha-core-StoreToImmutable: + +alpha.core.StoreToImmutable (C, C++) +"""""""""""""""""""""""""""""""""""" +Check for writes to immutable memory regions. This implements part of SEI CERT Rule ENV30-C. + +This checker detects attempts to write to memory regions that are marked as immutable, +including const variables, string literals, and other const-qualified memory. + +.. literalinclude:: checkers/storetoimmutable_example.cpp + :language: cpp + +**Solution** + +Avoid writing to const-qualified memory regions. If you need to modify the data, +remove the const qualifier from the original declaration or use a mutable copy. + alpha.cplusplus ^^^^^^^^^^^^^^^ diff --git a/clang/docs/analyzer/checkers/storetoimmutable_example.cpp b/clang/docs/analyzer/checkers/storetoimmutable_example.cpp new file mode 100644 index 0000000000000..bfd67ef7de96d --- /dev/null +++ b/clang/docs/analyzer/checkers/storetoimmutable_example.cpp @@ -0,0 +1,21 @@ +const int global_const = 42; + +struct TestStruct { + const int x; + int y; +}; + +void immutable_violation_examples() { + *(int *)&global_const = 100; // warn: Trying to write to immutable memory + + const int local_const = 42; + *(int *)&local_const = 43; // warn: Trying to write to immutable memory + + // NOTE: The following is reported in C++, but not in C, as the analyzer + // treats string literals as non-const char arrays in C mode. + char *ptr_to_str_literal = (char *)"hello"; + ptr_to_str_literal[0] = 'H'; // warn: Trying to write to immutable memory + + TestStruct s = {1, 2}; + *(int *)&s.x = 10; // warn: Trying to write to immutable memory +} diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 36196cdf62c6f..73f702de581d9 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -297,6 +297,11 @@ def StackAddrAsyncEscapeChecker "Check that addresses to stack memory do not escape the function">, Documentation; +def StoreToImmutableChecker : Checker<"StoreToImmutable">, + HelpText<"Check for writes to immutable memory regions. " + "This implements part of SEI CERT Rule ENV30-C.">, + Documentation; + def PthreadLockBase : Checker<"PthreadLockBase">, HelpText<"Helper registering multiple checks.">, Documentation, diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 22dd3f0374849..78360418a8b81 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -104,6 +104,7 @@ add_clang_library(clangStaticAnalyzerCheckers SmartPtrChecker.cpp SmartPtrModeling.cpp StackAddrEscapeChecker.cpp + StoreToImmutableChecker.cpp StdLibraryFunctionsChecker.cpp StdVariantChecker.cpp STLAlgorithmModeling.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp new file mode 100644 index 0000000000000..afad41939cdca --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp @@ -0,0 +1,188 @@ +//=== StoreToImmutableChecker.cpp - Store to immutable memory ---*- 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, a checker that detects writes +// to immutable memory regions. This implements part of SEI CERT Rule ENV30-C. +// +//===----------------------------------------------------------------------===// + +#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" +#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" + +using namespace clang; +using namespace ento; + +namespace { +class StoreToImmutableChecker : public Checker { + const BugType BT{this, "Write to immutable memory", "CERT Environment (ENV)"}; + +public: + void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const; +}; +} // end anonymous namespace + +static bool isInitializationContext(const Stmt *S, CheckerContext &C) { + // Check if this is a DeclStmt (variable declaration) + if (isa(S)) + return true; + + // This part is specific for initialization of const lambdas pre-C++17. + // Lets look at the AST of the statement: + // ``` + // const auto lambda = [](){}; + // ``` + // + // The relevant part of the AST for this case prior to C++17 is: + // ... + // `-DeclStmt + // `-VarDecl + // `-ExprWithCleanups + // `-CXXConstructExpr + // ... + // In C++17 and later, the AST is different: + // ... + // `-DeclStmt + // `-VarDecl + // `-ImplicitCastExpr + // `-LambdaExpr + // |-CXXRecordDecl + // `-CXXConstructExpr + // ... + // And even beside this, the statement `S` that is given to the checkBind + // callback is the VarDecl in C++17 and later, and the CXXConstructExpr in + // C++14 and before. So in order to support the C++14 we need the following + // ugly hack to detect whether this construction is used to initialize a + // variable. + // + // FIXME: This should be eliminated by improving the API of checkBind to + // ensure that it consistently passes the `VarDecl` (instead of the + // `CXXConstructExpr`) when the constructor call denotes the initialization + // of a variable with a lambda, or maybe less preferably, try the more + // invasive approach of passing the information forward to the checkers + // whether the current bind is an initialization or an assignment. + const auto *ConstructExp = dyn_cast(S); + return ConstructExp && ConstructExp->isElidable(); +} + +static bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) { + if (isa(MR)) + return true; + + // Check if this is a TypedRegion with a const-qualified type + if (const auto *TR = dyn_cast(MR)) { + QualType LocationType = TR->getDesugaredLocationType(C.getASTContext()); + if (LocationType->isPointerOrReferenceType()) + LocationType = LocationType->getPointeeType(); + if (LocationType.isConstQualified()) + return true; + } + + // Check if this is a SymbolicRegion with a const-qualified pointee type + if (const auto *SR = dyn_cast(MR)) { + QualType PointeeType = SR->getPointeeStaticType(); + if (PointeeType.isConstQualified()) + return true; + } + + // NOTE: The above branches do not cover AllocaRegion. We do not need to check + // AllocaRegion, as it models untyped memory, that is allocated on the stack. + + return false; +} + +static const MemRegion *getInnermostConstRegion(const MemRegion *MR, + CheckerContext &C) { + while (true) { + if (isEffectivelyConstRegion(MR, C)) + return MR; + if (auto *SR = dyn_cast(MR)) + MR = SR->getSuperRegion(); + else + return nullptr; + } +} + +static const DeclRegion * +getInnermostEnclosingConstDeclRegion(const MemRegion *MR, CheckerContext &C) { + while (true) { + if (const auto *DR = dyn_cast(MR)) { + const ValueDecl *D = DR->getDecl(); + QualType DeclaredType = D->getType(); + if (DeclaredType.isConstQualified()) + return DR; + } + if (auto *SR = dyn_cast(MR)) + MR = SR->getSuperRegion(); + else + return nullptr; + } +} + +void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S, + CheckerContext &C) const { + // We are only interested in stores to memory regions + const MemRegion *MR = Loc.getAsRegion(); + if (!MR) + return; + + // Skip variable declarations and initializations - we only want to catch + // actual writes + // FIXME: If the API of checkBind would allow to distinguish between + // initialization and assignment, we could use that instead. + if (isInitializationContext(S, C)) + return; + + // Check if the region is in the global immutable space + const MemSpaceRegion *MS = MR->getMemorySpace(C.getState()); + const bool IsGlobalImmutableSpace = isa(MS); + // Check if the region corresponds to a const variable + const MemRegion *InnermostConstRegion = getInnermostConstRegion(MR, C); + if (!IsGlobalImmutableSpace && !InnermostConstRegion) + return; + + SmallString<64> WarningMessage{"Trying to write to immutable memory"}; + if (IsGlobalImmutableSpace) + WarningMessage += " in global read-only storage"; + + // Generate the bug report + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; + + auto R = std::make_unique(BT, WarningMessage, N); + R->addRange(S->getSourceRange()); + + // Generate a note if the location that is being written to has a + // declaration or if it is a subregion of a const region with a declaration. + const DeclRegion *DR = + getInnermostEnclosingConstDeclRegion(InnermostConstRegion, C); + if (DR) { + const char *NoteMessage = + (DR != MR) ? "Enclosing memory region is declared as immutable here" + : "Memory region is declared as immutable here"; + R->addNote(NoteMessage, PathDiagnosticLocation::create( + DR->getDecl(), C.getSourceManager())); + } + + // For this checker, we are only interested in the value being written, no + // need to mark the value being assigned interesting. + + C.emitReport(std::move(R)); +} + +void ento::registerStoreToImmutableChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterStoreToImmutableChecker(const CheckerManager &mgr) { + return true; +} diff --git a/clang/test/Analysis/store-to-immutable-basic.c b/clang/test/Analysis/store-to-immutable-basic.c new file mode 100644 index 0000000000000..f7d887819714f --- /dev/null +++ b/clang/test/Analysis/store-to-immutable-basic.c @@ -0,0 +1,121 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -verify %s + +// Test basic functionality of StoreToImmutable checker for the C programming language. + +const int tentative_global_const; // expected-note {{Memory region is declared as immutable here}} + +void test_direct_write_to_tentative_const_global() { + *(int*)&tentative_global_const = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}} +} + +const int global_const = 42; // expected-note {{Memory region is declared as immutable here}} + +void test_direct_write_to_const_global() { + // This should trigger a warning about writing to immutable memory + *(int*)&global_const = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}} +} + +void test_write_through_const_pointer() { + const int local_const = 10; // expected-note {{Memory region is declared as immutable here}} + int *ptr = (int*)&local_const; + *ptr = 20; // expected-warning {{Trying to write to immutable memory}} +} + +void test_write_to_const_array() { + const int arr[5] = {1, 2, 3, 4, 5}; // expected-note {{Enclosing memory region is declared as immutable here}} + int *ptr = (int*)arr; + ptr[0] = 10; // expected-warning {{Trying to write to immutable memory}} +} + +struct TestStruct { + const int x; // expected-note 2 {{Memory region is declared as immutable here}} + int y; +}; + +void test_write_to_const_struct_member() { + struct TestStruct s = {1, 2}; + int *ptr = (int*)&s.x; + *ptr = 10; // expected-warning {{Trying to write to immutable memory}} +} + +const int global_array[3] = {1, 2, 3}; // expected-note {{Enclosing memory region is declared as immutable here}} + +void test_write_to_const_global_array() { + int *ptr = (int*)global_array; + ptr[0] = 10; // expected-warning {{Trying to write to immutable memory in global read-only storage}} +} + +const struct TestStruct global_struct = {1, 2}; + +void test_write_to_const_global_struct() { + int *ptr = (int*)&global_struct.x; + *ptr = 10; // expected-warning {{Trying to write to immutable memory in global read-only storage}} +} + + +void test_write_to_const_param(const int param) { // expected-note {{Memory region is declared as immutable here}} + *(int*)¶m = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_write_to_const_ptr_param(const int *param) { + *(int*)param = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_write_to_const_array_param(const int arr[5]) { + *(int*)arr = 100; // expected-warning {{Trying to write to immutable memory}} +} + +struct ParamStruct { + const int z; // expected-note 2 {{Memory region is declared as immutable here}} + int w; +}; + +void test_write_to_const_struct_param(const struct ParamStruct s) { + *(int*)&s.z = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_write_to_const_struct_ptr_param(const struct ParamStruct *s) { + *(int*)&s->z = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_write_to_nonconst() { + int non_const = 42; + *(int*)&non_const = 100; // No warning expected +} + +int global_non_const = 42; + +void test_write_to_nonconst_global() { + *(int*)&global_non_const = 100; // No warning expected +} + +struct NonConstStruct { + int x; + int y; +}; + +void test_write_to_nonconst_struct_member() { + struct NonConstStruct s = {1, 2}; + *(int*)&s.x = 100; // No warning expected +} + +void test_write_to_nonconst_param(int param) { + *(int*)¶m = 100; // No warning expected +} + +void test_normal_assignment() { + int x = 42; + x = 100; // No warning expected +} + +void test_const_ptr_to_nonconst_data() { + int data = 42; + const int *ptr = &data; + *(int*)ptr = 100; // No warning expected +} + +void test_const_ptr_to_const_data() { + const int data = 42; // expected-note {{Memory region is declared as immutable here}} + const int *ptr = &data; + *(int*)ptr = 100; // expected-warning {{Trying to write to immutable memory}} +} diff --git a/clang/test/Analysis/store-to-immutable-basic.cpp b/clang/test/Analysis/store-to-immutable-basic.cpp new file mode 100644 index 0000000000000..63319d9dfb7b1 --- /dev/null +++ b/clang/test/Analysis/store-to-immutable-basic.cpp @@ -0,0 +1,72 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++17 -verify %s + +void test_write_to_const_ref_param(const int ¶m) { + *(int*)¶m = 100; // expected-warning {{Trying to write to immutable memory}} +} + +// FIXME: This should warn in C mode too. +void test_write_to_string_literal() { + char *str = (char*)"hello"; + str[0] = 'H'; // expected-warning {{Trying to write to immutable memory}} +} + +struct ParamStruct { + const int z; // expected-note {{Memory region is declared as immutable here}} + int w; +}; + +void test_write_to_const_struct_ref_param(const ParamStruct &s) { + *(int*)&s.z = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_const_ref_to_nonconst_data() { + int data = 42; + const int &ref = data; + *(int*)&ref = 100; // No warning expected +} + +void test_const_ref_to_const_data() { + const int data = 42; // expected-note {{Memory region is declared as immutable here}} + const int &ref = data; + *(int*)&ref = 100; // expected-warning {{Trying to write to immutable memory}} +} + +void test_ref_to_nonconst_data() { + int data = 42; + int &ref = data; + ref = 100; // No warning expected +} + +void test_ref_to_const_data() { + const int data = 42; // expected-note {{Memory region is declared as immutable here}} + int &ref = *(int*)&data; + ref = 100; // expected-warning {{Trying to write to immutable memory}} +} + +struct MultipleLayerStruct { + MultipleLayerStruct(); + const int data; // expected-note {{Memory region is declared as immutable here}} + const int buf[10]; // expected-note {{Enclosing memory region is declared as immutable here}} +}; + +MultipleLayerStruct MLS[10]; + +void test_multiple_layer_struct_array_member() { + int *p = (int*)&MLS[2].data; + *p = 4; // expected-warning {{Trying to write to immutable memory}} +} + +void test_multiple_layer_struct_array_array_member() { + int *p = (int*)&MLS[2].buf[3]; + *p = 4; // expected-warning {{Trying to write to immutable memory}} +} + +struct StructWithNonConstMember { + int x; +}; + +const StructWithNonConstMember SWNCM{0}; // expected-note {{Enclosing memory region is declared as immutable here}} + +void test_write_to_non_const_member_of_const_struct() { + *(int*)&SWNCM.x = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}} +} diff --git a/clang/test/Analysis/store-to-immutable-lambda-init.cpp b/clang/test/Analysis/store-to-immutable-lambda-init.cpp new file mode 100644 index 0000000000000..764ce3fdbeb50 --- /dev/null +++ b/clang/test/Analysis/store-to-immutable-lambda-init.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++14 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++17 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++20 -verify %s + +// expected-no-diagnostics + +// In C++14 and before, when initializing a lambda, the statement given in the checkBind callback is not the whole DeclExpr, but the CXXConstructExpr of the lambda object. +// FIXME: Once the API of checkBind provides more information about the statement, the checker should be simplified, and this test case will no longer be a cornercase in the checker. + +void test_const_lambda_initialization_pre_cpp17() { + const auto lambda = [](){}; // No warning expected +}