Author: szelethus Date: Mon Feb 11 05:46:43 2019 New Revision: 353698 URL: http://llvm.org/viewvc/llvm-project?rev=353698&view=rev Log: [analyzer] New checker for detecting usages of unsafe I/O functions
There are certain unsafe or deprecated (since C11) buffer handling functions which should be avoided in safety critical code. They could cause buffer overflows. A new checker, 'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' warns for every occurrence of such functions (unsafe or deprecated printf, scanf family, and other buffer handling functions, which now have a secure variant). Patch by Dániel Kolozsvári! Differential Revision: https://reviews.llvm.org/D35068 Modified: cfe/trunk/docs/analyzer/checkers.rst cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp cfe/trunk/test/Analysis/security-syntax-checks.m Modified: cfe/trunk/docs/analyzer/checkers.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/checkers.rst?rev=353698&r1=353697&r2=353698&view=diff ============================================================================== --- cfe/trunk/docs/analyzer/checkers.rst (original) +++ cfe/trunk/docs/analyzer/checkers.rst Mon Feb 11 05:46:43 2019 @@ -566,6 +566,17 @@ security.insecureAPI.vfork (C) vfork(); // warn } +security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C) +"""""""""""""""""""""""""""""" + Warn on occurrences of unsafe or deprecated buffer handling functions, which now have a secure variant: ``sprintf, vsprintf, scanf, wscanf, fscanf, fwscanf, vscanf, vwscanf, vfscanf, vfwscanf, sscanf, swscanf, vsscanf, vswscanf, swprintf, snprintf, vswprintf, vsnprintf, memcpy, memmove, strncpy, strncat, memset`` + +.. code-block:: c + + void test() { + char buf [5]; + strncpy(buf, "a", 1); // warn + } + .. _unix-checkers: unix Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=353698&r1=353697&r2=353698&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Mon Feb 11 05:46:43 2019 @@ -622,6 +622,13 @@ def UncheckedReturn : Checker<"Unchecked Dependencies<[SecuritySyntaxChecker]>, Documentation<HasDocumentation>; +def DeprecatedOrUnsafeBufferHandling : + Checker<"DeprecatedOrUnsafeBufferHandling">, + HelpText<"Warn on uses of unsecure or deprecated buffer manipulating " + "functions">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation<HasDocumentation>; + } // end "security.insecureAPI" let ParentPackage = Security in { Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp?rev=353698&r1=353697&r2=353698&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp Mon Feb 11 05:46:43 2019 @@ -44,6 +44,7 @@ struct ChecksFilter { DefaultBool check_mktemp; DefaultBool check_mkstemp; DefaultBool check_strcpy; + DefaultBool check_DeprecatedOrUnsafeBufferHandling; DefaultBool check_rand; DefaultBool check_vfork; DefaultBool check_FloatLoopCounter; @@ -57,6 +58,7 @@ struct ChecksFilter { CheckName checkName_mktemp; CheckName checkName_mkstemp; CheckName checkName_strcpy; + CheckName checkName_DeprecatedOrUnsafeBufferHandling; CheckName checkName_rand; CheckName checkName_vfork; CheckName checkName_FloatLoopCounter; @@ -103,6 +105,8 @@ public: void checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD); void checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD); void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD); + void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, + const FunctionDecl *FD); void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD); void checkCall_random(const CallExpr *CE, const FunctionDecl *FD); void checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD); @@ -148,6 +152,14 @@ void WalkAST::VisitCallExpr(CallExpr *CE .Case("mkstemps", &WalkAST::checkCall_mkstemp) .Cases("strcpy", "__strcpy_chk", &WalkAST::checkCall_strcpy) .Cases("strcat", "__strcat_chk", &WalkAST::checkCall_strcat) + .Cases("sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf", + "vscanf", "vwscanf", "vfscanf", "vfwscanf", + &WalkAST::checkDeprecatedOrUnsafeBufferHandling) + .Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf", + "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove", + &WalkAST::checkDeprecatedOrUnsafeBufferHandling) + .Cases("strncpy", "strncat", "memset", + &WalkAST::checkDeprecatedOrUnsafeBufferHandling) .Case("drand48", &WalkAST::checkCall_rand) .Case("erand48", &WalkAST::checkCall_rand) .Case("jrand48", &WalkAST::checkCall_rand) @@ -552,7 +564,6 @@ void WalkAST::checkCall_mktemp(const Cal CELoc, CE->getCallee()->getSourceRange()); } - //===----------------------------------------------------------------------===// // Check: Use of 'mkstemp', 'mktemp', 'mkdtemp' should contain at least 6 X's. //===----------------------------------------------------------------------===// @@ -641,6 +652,7 @@ void WalkAST::checkCall_mkstemp(const Ca // CWE-119: Improper Restriction of Operations within // the Bounds of a Memory Buffer //===----------------------------------------------------------------------===// + void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) { if (!filter.check_strcpy) return; @@ -679,6 +691,7 @@ void WalkAST::checkCall_strcpy(const Cal // CWE-119: Improper Restriction of Operations within // the Bounds of a Memory Buffer //===----------------------------------------------------------------------===// + void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) { if (!filter.check_strcpy) return; @@ -701,8 +714,88 @@ void WalkAST::checkCall_strcat(const Cal } //===----------------------------------------------------------------------===// +// Check: Any use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf', +// 'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf', +// 'swscanf', 'vsscanf', 'vswscanf', 'swprintf', 'snprintf', 'vswprintf', +// 'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset' +// is deprecated since C11. +// +// Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf','fscanf', +// 'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf', +// 'swscanf', 'vsscanf', 'vswscanf' without buffer limitations +// is insecure. +// +// CWE-119: Improper Restriction of Operations within +// the Bounds of a Memory Buffer +//===----------------------------------------------------------------------===// + +void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, + const FunctionDecl *FD) { + if (!filter.check_DeprecatedOrUnsafeBufferHandling) + return; + + if (!BR.getContext().getLangOpts().C11) + return; + + // Issue a warning. ArgIndex == -1: Deprecated but not unsafe (has size + // restrictions). + enum { DEPR_ONLY = -1, UNKNOWN_CALL = -2 }; + StringRef Name = FD->getIdentifier()->getName(); + int ArgIndex = + llvm::StringSwitch<int>(Name) + .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0) + .Cases("sprintf", "vsprintf", "fscanf", "fwscanf", "vfscanf", + "vfwscanf", "sscanf", "swscanf", "vsscanf", "vswscanf", 1) + .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy", + "memmove", "memset", "strncpy", "strncat", DEPR_ONLY) + .Default(UNKNOWN_CALL); + + assert(ArgIndex != UNKNOWN_CALL && "Unsupported function"); + bool BoundsProvided = ArgIndex == DEPR_ONLY; + + if (!BoundsProvided) { + // Currently we only handle (not wide) string literals. It is possible to do + // better, either by looking at references to const variables, or by doing + // real flow analysis. + auto FormatString = + dyn_cast<StringLiteral>(CE->getArg(ArgIndex)->IgnoreParenImpCasts()); + if (FormatString && + FormatString->getString().find("%s") == StringRef::npos && + FormatString->getString().find("%[") == StringRef::npos) + BoundsProvided = true; + } + + SmallString<128> Buf1; + SmallString<512> Buf2; + llvm::raw_svector_ostream Out1(Buf1); + llvm::raw_svector_ostream Out2(Buf2); + + Out1 << "Potential insecure memory buffer bounds restriction in call '" + << Name << "'"; + Out2 << "Call to function '" << Name + << "' is insecure as it does not provide "; + + if (!BoundsProvided) { + Out2 << "bounding of the memory buffer or "; + } + + Out2 << "security checks introduced " + "in the C11 standard. Replace with analogous functions that " + "support length arguments or provides boundary checks such as '" + << Name << "_s' in case of C11"; + + PathDiagnosticLocation CELoc = + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), + filter.checkName_DeprecatedOrUnsafeBufferHandling, + Out1.str(), "Security", Out2.str(), CELoc, + CE->getCallee()->getSourceRange()); +} + +//===----------------------------------------------------------------------===// // Common check for str* functions with no bounds parameters. //===----------------------------------------------------------------------===// + bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) { const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>(); if (!FPT) @@ -936,5 +1029,4 @@ REGISTER_CHECKER(rand) REGISTER_CHECKER(vfork) REGISTER_CHECKER(FloatLoopCounter) REGISTER_CHECKER(UncheckedReturn) - - +REGISTER_CHECKER(DeprecatedOrUnsafeBufferHandling) Modified: cfe/trunk/test/Analysis/security-syntax-checks.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/security-syntax-checks.m?rev=353698&r1=353697&r2=353698&view=diff ============================================================================== --- cfe/trunk/test/Analysis/security-syntax-checks.m (original) +++ cfe/trunk/test/Analysis/security-syntax-checks.m Mon Feb 11 05:46:43 2019 @@ -13,6 +13,9 @@ # define BUILTIN(f) f #endif /* USE_BUILTINS */ +#include "Inputs/system-header-simulator-for-valist.h" +#include "Inputs/system-header-simulator-for-simple-stream.h" + typedef typeof(sizeof(int)) size_t; @@ -238,3 +241,82 @@ void test_mkstemp() { mkdtemp("XXXXXX"); } + +//===----------------------------------------------------------------------=== +// deprecated or unsafe buffer handling +//===----------------------------------------------------------------------=== +typedef int wchar_t; + +int sprintf(char *str, const char *format, ...); +//int vsprintf (char *s, const char *format, va_list arg); +int scanf(const char *format, ...); +int wscanf(const wchar_t *format, ...); +int fscanf(FILE *stream, const char *format, ...); +int fwscanf(FILE *stream, const wchar_t *format, ...); +int vscanf(const char *format, va_list arg); +int vwscanf(const wchar_t *format, va_list arg); +int vfscanf(FILE *stream, const char *format, va_list arg); +int vfwscanf(FILE *stream, const wchar_t *format, va_list arg); +int sscanf(const char *s, const char *format, ...); +int swscanf(const wchar_t *ws, const wchar_t *format, ...); +int vsscanf(const char *s, const char *format, va_list arg); +int vswscanf(const wchar_t *ws, const wchar_t *format, va_list arg); +int swprintf(wchar_t *ws, size_t len, const wchar_t *format, ...); +int snprintf(char *s, size_t n, const char *format, ...); +int vswprintf(wchar_t *ws, size_t len, const wchar_t *format, va_list arg); +int vsnprintf(char *s, size_t n, const char *format, va_list arg); +void *memcpy(void *destination, const void *source, size_t num); +void *memmove(void *destination, const void *source, size_t num); +char *strncpy(char *destination, const char *source, size_t num); +char *strncat(char *destination, const char *source, size_t num); +void *memset(void *ptr, int value, size_t num); + +void test_deprecated_or_unsafe_buffer_handling_1() { + char buf [5]; + wchar_t wbuf [5]; + int a; + FILE *file; + sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is insecure}} + scanf("%d", &a); // expected-warning{{Call to function 'scanf' is insecure}} + scanf("%s", buf); // expected-warning{{Call to function 'scanf' is insecure}} + scanf("%4s", buf); // expected-warning{{Call to function 'scanf' is insecure}} + wscanf((const wchar_t*) L"%s", buf); // expected-warning{{Call to function 'wscanf' is insecure}} + fscanf(file, "%d", &a); // expected-warning{{Call to function 'fscanf' is insecure}} + fscanf(file, "%s", buf); // expected-warning{{Call to function 'fscanf' is insecure}} + fscanf(file, "%4s", buf); // expected-warning{{Call to function 'fscanf' is insecure}} + fwscanf(file, (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'fwscanf' is insecure}} + sscanf("5", "%d", &a); // expected-warning{{Call to function 'sscanf' is insecure}} + sscanf("5", "%s", buf); // expected-warning{{Call to function 'sscanf' is insecure}} + sscanf("5", "%4s", buf); // expected-warning{{Call to function 'sscanf' is insecure}} + swscanf(L"5", (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'swscanf' is insecure}} + swprintf(L"5", 1, (const wchar_t*) L"%s", wbuf); // expected-warning{{Call to function 'swprintf' is insecure}} + snprintf("5", 1, "%s", buf); // expected-warning{{Call to function 'snprintf' is insecure}} + memcpy(buf, wbuf, 1); // expected-warning{{Call to function 'memcpy' is insecure}} + memmove(buf, wbuf, 1); // expected-warning{{Call to function 'memmove' is insecure}} + strncpy(buf, "a", 1); // expected-warning{{Call to function 'strncpy' is insecure}} + strncat(buf, "a", 1); // expected-warning{{Call to function 'strncat' is insecure}} + memset(buf, 'a', 1); // expected-warning{{Call to function 'memset' is insecure}} +} + +void test_deprecated_or_unsafe_buffer_handling_2(const char *format, ...) { + char buf [5]; + FILE *file; + va_list args; + va_start(args, format); + vsprintf(buf, format, args); // expected-warning{{Call to function 'vsprintf' is insecure}} + vscanf(format, args); // expected-warning{{Call to function 'vscanf' is insecure}} + vfscanf(file, format, args); // expected-warning{{Call to function 'vfscanf' is insecure}} + vsscanf("a", format, args); // expected-warning{{Call to function 'vsscanf' is insecure}} + vsnprintf("a", 1, format, args); // expected-warning{{Call to function 'vsnprintf' is insecure}} +} + +void test_deprecated_or_unsafe_buffer_handling_3(const wchar_t *format, ...) { + wchar_t wbuf [5]; + FILE *file; + va_list args; + va_start(args, format); + vwscanf(format, args); // expected-warning{{Call to function 'vwscanf' is insecure}} + vfwscanf(file, format, args); // expected-warning{{Call to function 'vfwscanf' is insecure}} + vswscanf(L"a", format, args); // expected-warning{{Call to function 'vswscanf' is insecure}} + vswprintf(L"a", 1, format, args); // expected-warning{{Call to function 'vswprintf' is insecure}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
