Skip to content

Commit ae704d8

Browse files
Balazs BenicshaoNoQ
authored andcommitted
[analyzer] Introduce the assume-controlled-environment config option
If the `assume-controlled-environment` is `true`, we should expect `getenv()` to succeed, and the result should not be considered tainted. By default, the option will be `false`. Reviewed By: NoQ, martong Differential Revision: https://reviews.llvm.org/D111296 (cherry picked from commit edde4ef) (cherry picked from commit 405bd5166cfc5dd9771ea5d651c9b2a76c6551bc)
1 parent 693fe1e commit ae704d8

File tree

5 files changed

+60
-12
lines changed

5 files changed

+60
-12
lines changed

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,14 @@ ANALYZER_OPTION(
328328
"holds a single element.",
329329
false)
330330

331+
ANALYZER_OPTION(
332+
bool, ShouldAssumeControlledEnvironment, "assume-controlled-environment",
333+
"Whether the analyzed application runs in a controlled environment. "
334+
"We will assume that environment variables exist in queries and they hold "
335+
"no malicious data. For instance, if this option is enabled, 'getenv()' "
336+
"might be modeled by the analyzer to never return NULL.",
337+
false)
338+
331339
//===----------------------------------------------------------------------===//
332340
// Unsigned analyzer options.
333341
//===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,6 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
435435
.Case("getch", {{}, {ReturnValueIndex}})
436436
.Case("getchar", {{}, {ReturnValueIndex}})
437437
.Case("getchar_unlocked", {{}, {ReturnValueIndex}})
438-
.Case("getenv", {{}, {ReturnValueIndex}})
439438
.Case("gets", {{}, {0, ReturnValueIndex}})
440439
.Case("scanf", {{}, {}, VariadicType::Dst, 1})
441440
.Case("socket", {{},
@@ -468,6 +467,16 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
468467

469468
if (!Rule.isNull())
470469
return Rule;
470+
471+
// `getenv` returns taint only in untrusted environments.
472+
if (FData.FullName == "getenv") {
473+
if (C.getAnalysisManager()
474+
.getAnalyzerOptions()
475+
.ShouldAssumeControlledEnvironment)
476+
return {};
477+
return {{}, {ReturnValueIndex}};
478+
}
479+
471480
assert(FData.FDecl);
472481

473482
// Check if it's one of the memory setting/copying functions.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@ class StdLibraryFunctionsChecker
568568

569569
bool DisplayLoadedSummaries = false;
570570
bool ModelPOSIX = false;
571+
bool ShouldAssumeControlledEnvironment = false;
571572

572573
private:
573574
Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
@@ -1433,13 +1434,19 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
14331434
RetType{Ssize_tTy}),
14341435
GetLineSummary);
14351436

1436-
// char *getenv(const char *name);
1437-
addToFunctionSummaryMap(
1438-
"getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}),
1439-
Summary(NoEvalCall)
1440-
.Case({NotNull(Ret)})
1441-
.Case({NotNull(Ret)->negate()})
1442-
.ArgConstraint(NotNull(ArgNo(0))));
1437+
{
1438+
Summary GetenvSummary = Summary(NoEvalCall)
1439+
.ArgConstraint(NotNull(ArgNo(0)))
1440+
.Case({NotNull(Ret)});
1441+
// In untrusted environments the envvar might not exist.
1442+
if (!ShouldAssumeControlledEnvironment)
1443+
GetenvSummary.Case({NotNull(Ret)->negate()});
1444+
1445+
// char *getenv(const char *name);
1446+
addToFunctionSummaryMap(
1447+
"getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}),
1448+
std::move(GetenvSummary));
1449+
}
14431450

14441451
if (ModelPOSIX) {
14451452

@@ -2653,11 +2660,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
26532660

26542661
void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) {
26552662
auto *Checker = mgr.registerChecker<StdLibraryFunctionsChecker>();
2663+
const AnalyzerOptions &Opts = mgr.getAnalyzerOptions();
26562664
Checker->DisplayLoadedSummaries =
2657-
mgr.getAnalyzerOptions().getCheckerBooleanOption(
2658-
Checker, "DisplayLoadedSummaries");
2659-
Checker->ModelPOSIX =
2660-
mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "ModelPOSIX");
2665+
Opts.getCheckerBooleanOption(Checker, "DisplayLoadedSummaries");
2666+
Checker->ModelPOSIX = Opts.getCheckerBooleanOption(Checker, "ModelPOSIX");
2667+
Checker->ShouldAssumeControlledEnvironment =
2668+
Opts.ShouldAssumeControlledEnvironment;
26612669
}
26622670

26632671
bool ento::shouldRegisterStdCLibraryFunctionsChecker(

clang/test/Analysis/analyzer-config.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries = false
1616
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:ModelPOSIX = false
1717
// CHECK-NEXT: apply-fixits = false
18+
// CHECK-NEXT: assume-controlled-environment = false
1819
// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
1920
// CHECK-NEXT: c++-allocator-inlining = true
2021
// CHECK-NEXT: c++-container-inlining = false
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %clang_analyze_cc1 -verify=untrusted-env %s \
2+
// RUN: -analyzer-checker=core \
3+
// RUN: -analyzer-checker=alpha.security.taint \
4+
// RUN: -analyzer-checker=debug.TaintTest
5+
6+
// RUN: %clang_analyze_cc1 -verify %s -DEXPECT_NO_WARNINGS \
7+
// RUN: -analyzer-config assume-controlled-environment=true \
8+
// RUN: -analyzer-checker=core \
9+
// RUN: -analyzer-checker=alpha.security.taint \
10+
// RUN: -analyzer-checker=debug.TaintTest
11+
12+
13+
#ifdef EXPECT_NO_WARNINGS
14+
// expected-no-diagnostics
15+
#endif
16+
17+
char *getenv(const char *name);
18+
19+
void foo() {
20+
char *p = getenv("FOO"); // untrusted-env-warning {{tainted}}
21+
(void)p; // untrusted-env-warning {{tainted}}
22+
}

0 commit comments

Comments
 (0)