diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -663,11 +663,29 @@ for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // warn } +security.UncheckedReturnValue +""""""""""""""""""""""""""""" +Warn on uses of functions whose return values must be checked (CERT: ERR33-C). +The detection is limited to the case when the return value of the function +is completely ignored. The detection is applied in C++ code too. +List of checked functions is: +FIXME: Fill the list. + +.. code-block:: c + + void f(); + + void test() { + atexit(f); // warn: return value of atexit should be checked + } + .. _security-insecureAPI-UncheckedReturn: security.insecureAPI.UncheckedReturn (C) """""""""""""""""""""""""""""""""""""""" Warn on uses of functions whose return values must be always checked. +This check detects ignored return value at calls to +'setuid', 'setgid', 'seteuid', 'setegid', 'setreuid', 'setregid'. .. code-block:: c diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -926,6 +926,11 @@ Dependencies<[SecuritySyntaxChecker]>, Documentation; +def UncheckedReturnValueChecker : Checker<"UncheckedReturnValue">, + HelpText<"Warn on uses of functions whose return values must be always " + "checked.">, + Documentation; + } // end "security" let ParentPackage = POS in { diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -109,6 +109,7 @@ TestAfterDivZeroChecker.cpp TraversalChecker.cpp TrustNonnullChecker.cpp + UncheckedReturnValueChecker.cpp UndefBranchChecker.cpp UndefCapturedBlockVarChecker.cpp UndefResultChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp @@ -0,0 +1,178 @@ +//===- ReturnValueChecker - Applies guaranteed return values ----*- 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 defines UncheckedReturnValueChecker, which checks for calls to functions +// whose return value has to be observed by the program but is ignored. +// +//===----------------------------------------------------------------------===// + +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" + +using namespace clang; +using namespace ento; +using namespace ast_matchers; + +namespace { + +static bool isInNoNamespace(const FunctionDecl *FD) { + const DeclContext *DC = FD->getDeclContext(); + while (DC && DC->isTransparentContext()) + DC = DC->getParent(); + assert(DC && "Function should have a declaration context."); + return DC->isTranslationUnit(); +} + +class UncheckedReturnValueChecker : public Checker { +public: + void checkASTCodeBody(const Decl *D, AnalysisManager &AM, + BugReporter &BR) const { + auto FoundCall = callExpr().bind("call"); + auto CallInCompound = compoundStmt(forEach(FoundCall)); + auto CallInIf = ifStmt(anyOf(hasThen(FoundCall), hasElse(FoundCall))); + auto CallInFor = forStmt(anyOf(hasBody(FoundCall), hasLoopInit(FoundCall), + hasIncrement(FoundCall))); + auto CallInForRange = cxxForRangeStmt(hasBody(FoundCall)); + auto CallInWhile = whileStmt(hasBody(FoundCall)); + auto CallInDo = doStmt(hasBody(FoundCall)); + auto CallInCase = caseStmt(has(FoundCall)); + auto CallInAnyStmt = + stmt(anyOf(CallInCompound, CallInIf, CallInFor, CallInForRange, + CallInWhile, CallInDo, CallInCase)); + auto Matches = + match(stmt(findAll(CallInAnyStmt)), *D->getBody(), AM.getASTContext()); + + for (BoundNodes Match : Matches) { + const auto *CE = Match.getNodeAs("call"); + assert(CE); + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) + continue; + FD = FD->getCanonicalDecl(); + + if (!AM.getSourceManager().isInSystemHeader(FD->getLocation())) + continue; + if (!FD->isInStdNamespace() && !isInNoNamespace(FD)) + continue; + + if (isFunctionToCheck(FD)) { + // Issue a warning. + SmallString<512> buf; + llvm::raw_svector_ostream os(buf); + os << "Return value is not checked in call to '" << *FD << "'."; + + PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin( + CE, BR.getSourceManager(), AM.getAnalysisDeclContext(D)); + + BR.EmitBasicReport(D, getCheckerName(), "Unchecked return value", + categories::SecurityError, os.str(), CELoc); + } + } + } + +private: + llvm::StringMap FunctionsToCheck = { + {"aligned_alloc", 2}, {"asctime_s", 3}, {"at_quick_exit", 1}, + {"atexit", 1}, {"bsearch", 5}, {"bsearch_s", 6}, + {"btowc", 1}, {"c16rtomb", 3}, {"c32rtomb", 3}, + {"calloc", 2}, {"clock", 0}, {"cnd_broadcast", 1}, + {"cnd_init", 1}, {"cnd_signal", 1}, {"cnd_timedwait", 3}, + {"cnd_wait", 2}, {"ctime_s", 3}, {"fclose", 1}, + {"fflush", 1}, {"fgetc", 1}, {"fgetpos", 2}, + {"fgets", 3}, {"fgetwc", 1}, {"fopen", 2}, + {"fopen_s", 3}, {"fprintf", -1}, {"fprintf_s", -1}, + {"fputc", 2}, {"fputs", 2}, {"fputwc", 2}, + {"fputws", 2}, {"fread", 4}, {"freopen", 3}, + {"freopen_s", 4}, {"fscanf", -1}, {"fscanf_s", -1}, + {"fseek", 3}, {"fsetpos", 2}, {"ftell", 1}, + {"fwprintf", -1}, {"fwprintf_s", -1}, {"fwrite", 4}, + {"fwscanf", -1}, {"fwscanf_s", -1}, {"getc", 1}, + {"getchar", 0}, {"getenv", 1}, {"getenv_s", 4}, + {"gets_s", 2}, {"getwc", 1}, {"getwchar", 0}, + {"gmtime", 1}, {"gmtime_s", 2}, {"localtime", 1}, + {"localtime_s", 2}, {"malloc", 1}, {"mblen", 2}, + {"mbrlen", 3}, {"mbrtoc16", 4}, {"mbrtoc32", 4}, + {"mbrtowc", 4}, {"mbsrtowcs", 4}, {"mbsrtowcs_s", 6}, + {"mbstowcs", 3}, {"mbstowcs_s", 5}, {"mbtowc", 3}, + {"memchr", 3}, {"mktime", 1}, {"mtx_init", 2}, + {"mtx_lock", 1}, {"mtx_timedlock", 2}, {"mtx_trylock", 1}, + {"mtx_unlock", 1}, {"printf_s", -1}, {"putc", 2}, + {"putwc", 2}, {"raise", 1}, {"realloc", 2}, + {"remove", 1}, {"rename", 2}, {"setlocale", 2}, + {"setvbuf", 4}, {"scanf", -1}, {"scanf_s", -1}, + {"signal", 2}, {"snprintf", -1}, {"snprintf_s", -1}, + {"snwprintf_s", -1}, {"sprintf", -1}, {"sprintf_s", -1}, + {"sscanf", -1}, {"sscanf_s", -1}, {"strchr", 2}, + {"strerror_s", 3}, {"strftime", 4}, {"strpbrk", 2}, + {"strrchr", 2}, {"strstr", 2}, {"strtod", 2}, + {"strtof", 2}, {"strtoimax", 3}, {"strtok", 2}, + {"strtok_s", 4}, {"strtol", 3}, {"strtold", 2}, + {"strtoll", 3}, {"strtoumax", 3}, {"strtoul", 3}, + {"strtoull", 3}, {"strxfrm", 3}, {"swprintf", -1}, + {"swprintf_s", -1}, {"swscanf", -1}, {"swscanf_s", -1}, + {"thrd_create", 3}, {"thrd_detach", 1}, {"thrd_join", 2}, + {"thrd_sleep", 2}, {"time", 1}, {"timespec_get", 2}, + {"tmpfile", 0}, {"tmpfile_s", 1}, {"tmpnam", 1}, + {"tmpnam_s", 2}, {"tss_create", 2}, {"tss_get", 1}, + {"tss_set", 2}, {"ungetc", 2}, {"ungetwc", 2}, + {"vfprintf", 3}, {"vfprintf_s", 3}, {"vfscanf", 3}, + {"vfscanf_s", 3}, {"vfwprintf", 3}, {"vfwprintf_s", 3}, + {"vfwscanf", 3}, {"vfwscanf_s", 3}, {"vprintf", 2}, + {"vprintf_s", 2}, {"vscanf", 2}, {"vscanf_s", 2}, + {"vsnprintf", 4}, {"vsnprintf_s", 4}, {"vsnwprintf_s", 4}, + {"vsprintf", 3}, {"vsprintf_s", 4}, {"vsscanf", 3}, + {"vsscanf_s", 3}, {"vswprintf", 4}, {"vswprintf_s", 4}, + {"vswscanf", 3}, {"vswscanf_s", 3}, {"vwprintf_s", 2}, + {"vwscanf", 2}, {"vwscanf_s", 2}, {"wcrtomb", 3}, + {"wcrtomb_s", 5}, {"wcschr", 2}, {"wcsftime", 4}, + {"wcspbrk", 2}, {"wcsrchr", 2}, {"wcsrtombs", 4}, + {"wcsrtombs_s", 5}, {"wcsstr", 2}, {"wcstod", 2}, + {"wcstof", 2}, {"wcstoimax", 3}, {"wcstok", 3}, + {"wcstok_s", 4}, {"wcstol", 3}, {"wcstold", 2}, + {"wcstoll", 3}, {"wcstombs", 3}, {"wcstombs_s", 5}, + {"wcstoumax", 3}, {"wcstoul", 3}, {"wcstoull", 3}, + {"wcsxfrm", 3}, {"wctob", 1}, {"wctomb", 2}, + {"wctomb_s", 4}, {"wctrans", 1}, {"wctype", 1}, + {"wmemchr", 3}, {"wprintf_s", -1}, {"wscanf", -1}, + {"wscanf_s", -1}}; + + bool isFunctionToCheck(const FunctionDecl *FD) const { + // Ignore functions without a name. + const IdentifierInfo *II = FD->getIdentifier(); + if (!II) + return false; + + auto FoundFunction = FunctionsToCheck.find(II->getName()); + if (FoundFunction == FunctionsToCheck.end()) + return false; + + // Value -1 indicates that the function should be variadic. + if (FoundFunction->second == -1) + return FD->isVariadic(); + + // Check parameter count. + return FD->getNumParams() == static_cast(FoundFunction->second); + } +}; + +} // namespace + +void ento::registerUncheckedReturnValueChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterUncheckedReturnValueChecker( + const CheckerManager &mgr) { + return true; +} diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1139,4 +1139,10 @@ // TODO: Add some actual implementation. }; +int scanf(const char *format, ...); + } // namespace std + +namespace other_ns { +int scanf(const char *format, ...); +} diff --git a/clang/test/Analysis/unchecked-return-value.cpp b/clang/test/Analysis/unchecked-return-value.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/unchecked-return-value.cpp @@ -0,0 +1,76 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=security.UncheckedReturnValue -verify %s + +#include "Inputs/system-header-simulator-cxx.h" +#include "Inputs/system-header-simulator.h" + +extern void extern_f(int); +int btowc(int); + +struct S { + int getInt() const; +}; + +int f1(int X) { + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + std::scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + other_ns::scanf("%*d"); + { + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + } + + if (X) { + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + } + + if (X) + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + if (scanf("%*d")) { + } + if (scanf("%*d") > 1) { + } + + while (true) + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + while (scanf("%*d")) { + } + + do + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + while (true); + do { + } while (scanf("%*d")); + + for (int Y = 1; Y < 10; ++Y) + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + for (int Y = 1; Y < 10; scanf("%*d")) { // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + } + for (int Y = 1; scanf("%*d"); ++Y) { + } + int Y = 0; + for (scanf("%*d"); Y < 10; ++Y) { // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + } + + std::vector Vec; + for (int I : Vec) + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + + switch (scanf("%*d")) { + } + switch (X) { + case 1: + scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + break; + } + + // This call is wrapped in an ExprWithCleanups. + scanf("", S().getInt()); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}} + + int Z1 = scanf("%*d"); + Z1 = scanf("%*d"); + extern_f(scanf("%*d")); + + btowc(22); // no warning for non-system function + + return scanf("%*d"); // no warning if value is returned +}