gamesh411 updated this revision to Diff 556510.
gamesh411 added a comment.
- use std::string
- simplify tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154603/new/
https://reviews.llvm.org/D154603
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/cert/env34-c-cert-examples.c
clang/test/Analysis/cert/env34-c.c
clang/test/Analysis/invalid-ptr-checker.c
Index: clang/test/Analysis/invalid-ptr-checker.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/invalid-ptr-checker.c
@@ -0,0 +1,50 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: -analyzer-output=text -verify -Wno-unused %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config \
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=expected,pedantic -Wno-unused %s
+
+#include "Inputs/system-header-simulator.h"
+
+char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
+int strcmp(const char *, const char *);
+
+int custom_env_handler(const char **envp);
+
+void getenv_after_getenv(void) {
+ char *v1 = getenv("V1");
+ // pedantic-note@-1{{previous function call was here}}
+
+ char *v2 = getenv("V2");
+ // pedantic-note@-1{{'getenv' call may invalidate the result of the previous 'getenv'}}
+
+ strcmp(v1, v2);
+ // pedantic-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+ // pedantic-note@-2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+void setenv_after_getenv(void) {
+ char *v1 = getenv("VAR1");
+
+ setenv("VAR2", "...", 1);
+ // expected-note@-1{{'setenv' call may invalidate the environment returned by getenv}}
+
+ strcmp(v1, "");
+ // expected-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+ // expected-note@-2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+int main(int argc, const char *argv[], const char *envp[]) {
+ setenv("VAR", "...", 0);
+ // expected-note@-1 2 {{'setenv' call may invalidate the environment parameter of 'main'}}
+
+ *envp;
+ // expected-warning@-1 2 {{dereferencing an invalid pointer}}
+ // expected-note@-2 2 {{dereferencing an invalid pointer}}
+}
Index: clang/test/Analysis/cert/env34-c.c
===================================================================
--- clang/test/Analysis/cert/env34-c.c
+++ clang/test/Analysis/cert/env34-c.c
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr\
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
// RUN: -analyzer-output=text -verify -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
Index: clang/test/Analysis/cert/env34-c-cert-examples.c
===================================================================
--- clang/test/Analysis/cert/env34-c-cert-examples.c
+++ clang/test/Analysis/cert/env34-c-cert-examples.c
@@ -1,15 +1,49 @@
+// Default options.
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
// RUN: -verify -Wno-unused %s
+//
+// Test the laxer handling of getenv function (this is the default).
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: -verify -Wno-unused %s
+//
+// Test the stricter handling of getenv function.
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -verify=pedantic -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
int strcmp(const char*, const char*);
char *strdup(const char*);
void free(void *memblock);
void *malloc(size_t size);
-void incorrect_usage(void) {
+void incorrect_usage_setenv_getenv_invalidation(void) {
+ char *tmpvar;
+ char *tempvar;
+
+ tmpvar = getenv("TMP");
+
+ if (!tmpvar)
+ return;
+
+ setenv("TEMP", "", 1); //setenv can invalidate env
+
+ if (!tmpvar)
+ return;
+
+ if (strcmp(tmpvar, "") == 0) { // body of strcmp is unknown
+ // expected-warning@-1{{use of invalidated pointer 'tmpvar' in a function call}}
+ // pedantic-warning@-2{{use of invalidated pointer 'tmpvar' in a function call}}
+ }
+}
+
+void incorrect_usage_double_getenv_invalidation(void) {
char *tmpvar;
char *tempvar;
@@ -18,13 +52,13 @@
if (!tmpvar)
return;
- tempvar = getenv("TEMP");
+ tempvar = getenv("TEMP"); //getenv should not invalidate env in non-pedantic mode
if (!tempvar)
return;
if (strcmp(tmpvar, tempvar) == 0) { // body of strcmp is unknown
- // expected-warning@-1{{use of invalidated pointer 'tmpvar' in a function call}}
+ // pedantic-warning@-1{{use of invalidated pointer 'tmpvar' in a function call}}
}
}
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -11,6 +11,7 @@
// CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
+// CHECK-NEXT: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv = false
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: alpha.unix.StdCLibraryFunctions:DisplayLoadedSummaries = false
Index: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -38,6 +38,15 @@
CheckerContext &C) const;
// SEI CERT ENV31-C
+
+ // If set to true, consider getenv calls as invalidating operations on the
+ // environment variable buffer. This is implied in the standard, but in
+ // practice does not cause problems (in the commonly used environments).
+ bool InvalidatingGetEnv = false;
+
+ // GetEnv can be treated invalidating and non-invalidating as well.
+ const CallDescription GetEnvCall{{"getenv"}, 1};
+
const CallDescriptionMap<HandlerFn> EnvpInvalidatingFunctions = {
{{{"setenv"}, 3}, &InvalidPtrChecker::EnvpInvalidatingCall},
{{{"unsetenv"}, 1}, &InvalidPtrChecker::EnvpInvalidatingCall},
@@ -51,7 +60,6 @@
// SEI CERT ENV34-C
const CallDescriptionMap<HandlerFn> PreviousCallInvalidatingFunctions = {
- {{{"getenv"}, 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
{{{"setlocale"}, 2},
&InvalidPtrChecker::postPreviousReturnInvalidatingCall},
{{{"strerror"}, 1},
@@ -62,6 +70,10 @@
&InvalidPtrChecker::postPreviousReturnInvalidatingCall},
};
+ // The private members of this checker corresponding to commandline options
+ // are set in this function.
+ friend void ento::registerInvalidPtrChecker(CheckerManager &);
+
public:
// Obtain the environment pointer from 'main()' (if present).
void checkBeginFunction(CheckerContext &C) const;
@@ -84,7 +96,10 @@
REGISTER_SET_WITH_PROGRAMSTATE(InvalidMemoryRegions, const MemRegion *)
// Stores the region of the environment pointer of 'main' (if present).
-REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const MemRegion *)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(MainEnvPtrRegion, const MemRegion *)
+
+// Stores the regions of environments returned by getenv calls.
+REGISTER_SET_WITH_PROGRAMSTATE(GetenvEnvPtrRegions, const MemRegion *)
// Stores key-value pairs, where key is function declaration and value is
// pointer to memory region returned by previous call of this function
@@ -95,22 +110,35 @@
CheckerContext &C) const {
StringRef FunctionName = Call.getCalleeIdentifier()->getName();
ProgramStateRef State = C.getState();
- const MemRegion *SymbolicEnvPtrRegion = State->get<EnvPtrRegion>();
- if (!SymbolicEnvPtrRegion)
- return;
- State = State->add<InvalidMemoryRegions>(SymbolicEnvPtrRegion);
+ auto PlaceInvalidationNote = [&C, FunctionName,
+ &State](const MemRegion *Region,
+ StringRef Message, ExplodedNode *Pred) {
+ State = State->add<InvalidMemoryRegions>(Region);
+
+ // Make copy of string data for the time when notes are *actually* created.
+ const NoteTag *Note =
+ C.getNoteTag([Region, FunctionName = std::string{FunctionName},
+ Message = std::string{Message}](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ if (!BR.isInteresting(Region))
+ return;
+ Out << '\'' << FunctionName << "' " << Message;
+ });
+ return C.addTransition(State, Pred, Note);
+ };
- const NoteTag *Note =
- C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
- PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(SymbolicEnvPtrRegion))
- return;
- Out << '\'' << FunctionName
- << "' call may invalidate the environment parameter of 'main'";
- });
+ ExplodedNode *CurrentChainEnd = C.getPredecessor();
+
+ if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ MainEnvPtr, "call may invalidate the environment parameter of 'main'",
+ CurrentChainEnd);
- C.addTransition(State, Note);
+ for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ EnvPtr, "call may invalidate the environment returned by getenv",
+ CurrentChainEnd);
}
void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
@@ -146,8 +174,7 @@
// Remember to this region.
const auto *SymRegOfRetVal = cast<SymbolicRegion>(RetVal.getAsRegion());
- const MemRegion *MR =
- const_cast<MemRegion *>(SymRegOfRetVal->getBaseRegion());
+ const MemRegion *MR = SymRegOfRetVal->getBaseRegion();
State = State->set<PreviousCallResultMap>(FD, MR);
ExplodedNode *Node = C.addTransition(State, Note);
@@ -185,6 +212,18 @@
// function call as an argument.
void InvalidPtrChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
+
+ ProgramStateRef State = C.getState();
+
+ // Model 'getenv' calls
+ if (GetEnvCall.matches(Call)) {
+ const MemRegion *Region = Call.getReturnValue().getAsRegion();
+ if (Region) {
+ State = State->add<GetenvEnvPtrRegions>(Region);
+ C.addTransition(State);
+ }
+ }
+
// Check if function invalidates 'envp' argument of 'main'
if (const auto *Handler = EnvpInvalidatingFunctions.lookup(Call))
(this->**Handler)(Call, C);
@@ -193,14 +232,16 @@
if (const auto *Handler = PreviousCallInvalidatingFunctions.lookup(Call))
(this->**Handler)(Call, C);
+ // If pedantic mode is on, regard 'getenv' calls invalidating as well
+ if (InvalidatingGetEnv && GetEnvCall.matches(Call))
+ postPreviousReturnInvalidatingCall(Call, C);
+
// Check if one of the arguments of the function call is invalidated
// If call was inlined, don't report invalidated argument
if (C.wasInlined)
return;
- ProgramStateRef State = C.getState();
-
for (unsigned I = 0, NumArgs = Call.getNumArgs(); I < NumArgs; ++I) {
if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(
@@ -243,7 +284,7 @@
// Save the memory region pointed by the environment pointer parameter of
// 'main'.
- C.addTransition(State->set<EnvPtrRegion>(EnvpReg));
+ C.addTransition(State->set<MainEnvPtrRegion>(EnvpReg));
}
// Check if invalidated region is being dereferenced.
@@ -268,7 +309,10 @@
}
void ento::registerInvalidPtrChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<InvalidPtrChecker>();
+ auto *Checker = Mgr.registerChecker<InvalidPtrChecker>();
+ Checker->InvalidatingGetEnv =
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker,
+ "InvalidatingGetEnv");
}
bool ento::shouldRegisterInvalidPtrChecker(const CheckerManager &) {
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -997,6 +997,15 @@
def InvalidPtrChecker : Checker<"InvalidPtr">,
HelpText<"Finds usages of possibly invalidated pointers">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "InvalidatingGetEnv",
+ "Regard getenv as an invalidating call (as per POSIX "
+ "standard), which can lead to false positives depending on "
+ "implementation.",
+ "false",
+ InAlpha>,
+ ]>,
Documentation<HasDocumentation>;
} // end "alpha.cert.env"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
