From 768fb8ff51f82747c5df9b4708e8c1022e47cf7e Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Sat, 26 May 2018 00:04:26 +0000 Subject: [PATCH] [analyzer] Add security checks for bcmp(), bcopy(), bzero(). These functions are obsolete. The analyzer would advice to replace them with memcmp(), memcpy() or memmove(), or memset(). Patch by Tom Rix! Differential Revision: https://reviews.llvm.org/D41881 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@333326 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 9 ++ .../Checkers/CheckSecuritySyntaxOnly.cpp | 141 ++++++++++++++++++ test/Analysis/security-syntax-checks.m | 21 +++ www/analyzer/available_checks.html | 34 +++++ 4 files changed, 205 insertions(+) diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td index fdeeca0e37..eb91155d9d 100644 --- a/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -373,6 +373,15 @@ def PaddingChecker : Checker<"Padding">, //===----------------------------------------------------------------------===// let ParentPackage = InsecureAPI in { + def bcmp : Checker<"bcmp">, + HelpText<"Warn on uses of the 'bcmp' function">, + DescFile<"CheckSecuritySyntaxOnly.cpp">; + def bcopy : Checker<"bcopy">, + HelpText<"Warn on uses of the 'bcopy' function">, + DescFile<"CheckSecuritySyntaxOnly.cpp">; + def bzero : Checker<"bzero">, + HelpText<"Warn on uses of the 'bzero' function">, + DescFile<"CheckSecuritySyntaxOnly.cpp">; def gets : Checker<"gets">, HelpText<"Warn on uses of the 'gets' function">, DescFile<"CheckSecuritySyntaxOnly.cpp">; diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 62831be26e..15a13d8ac9 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -37,6 +37,9 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) { namespace { struct ChecksFilter { + DefaultBool check_bcmp; + DefaultBool check_bcopy; + DefaultBool check_bzero; DefaultBool check_gets; DefaultBool check_getpw; DefaultBool check_mktemp; @@ -47,6 +50,9 @@ struct ChecksFilter { DefaultBool check_FloatLoopCounter; DefaultBool check_UncheckedReturn; + CheckName checkName_bcmp; + CheckName checkName_bcopy; + CheckName checkName_bzero; CheckName checkName_gets; CheckName checkName_getpw; CheckName checkName_mktemp; @@ -89,6 +95,9 @@ public: // Checker-specific methods. void checkLoopConditionForFloat(const ForStmt *FS); + void checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD); + void checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD); + void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD); void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD); void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD); void checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD); @@ -129,6 +138,9 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { // Set the evaluation function by switching on the callee name. FnCheck evalFunction = llvm::StringSwitch(Name) + .Case("bcmp", &WalkAST::checkCall_bcmp) + .Case("bcopy", &WalkAST::checkCall_bcopy) + .Case("bzero", &WalkAST::checkCall_bzero) .Case("gets", &WalkAST::checkCall_gets) .Case("getpw", &WalkAST::checkCall_getpw) .Case("mktemp", &WalkAST::checkCall_mktemp) @@ -295,6 +307,132 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) { FSLoc, ranges); } +//===----------------------------------------------------------------------===// +// Check: Any use of bcmp. +// CWE-477: Use of Obsolete Functions +// bcmp was deprecated in POSIX.1-2008 +//===----------------------------------------------------------------------===// + +void WalkAST::checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD) { + if (!filter.check_bcmp) + return; + + const FunctionProtoType *FPT = FD->getType()->getAs(); + if (!FPT) + return; + + // Verify that the function takes three arguments. + if (FPT->getNumParams() != 3) + return; + + for (int i = 0; i < 2; i++) { + // Verify the first and second argument type is void*. + const PointerType *PT = FPT->getParamType(i)->getAs(); + if (!PT) + return; + + if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy) + return; + } + + // Verify the third argument type is integer. + if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType()) + return; + + // Issue a warning. + PathDiagnosticLocation CELoc = + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), filter.checkName_bcmp, + "Use of deprecated function in call to 'bcmp()'", + "Security", + "The bcmp() function is obsoleted by memcmp().", + CELoc, CE->getCallee()->getSourceRange()); +} + +//===----------------------------------------------------------------------===// +// Check: Any use of bcopy. +// CWE-477: Use of Obsolete Functions +// bcopy was deprecated in POSIX.1-2008 +//===----------------------------------------------------------------------===// + +void WalkAST::checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD) { + if (!filter.check_bcopy) + return; + + const FunctionProtoType *FPT = FD->getType()->getAs(); + if (!FPT) + return; + + // Verify that the function takes three arguments. + if (FPT->getNumParams() != 3) + return; + + for (int i = 0; i < 2; i++) { + // Verify the first and second argument type is void*. + const PointerType *PT = FPT->getParamType(i)->getAs(); + if (!PT) + return; + + if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy) + return; + } + + // Verify the third argument type is integer. + if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType()) + return; + + // Issue a warning. + PathDiagnosticLocation CELoc = + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), filter.checkName_bcopy, + "Use of deprecated function in call to 'bcopy()'", + "Security", + "The bcopy() function is obsoleted by memcpy() " + "or memmove().", + CELoc, CE->getCallee()->getSourceRange()); +} + +//===----------------------------------------------------------------------===// +// Check: Any use of bzero. +// CWE-477: Use of Obsolete Functions +// bzero was deprecated in POSIX.1-2008 +//===----------------------------------------------------------------------===// + +void WalkAST::checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD) { + if (!filter.check_bzero) + return; + + const FunctionProtoType *FPT = FD->getType()->getAs(); + if (!FPT) + return; + + // Verify that the function takes two arguments. + if (FPT->getNumParams() != 2) + return; + + // Verify the first argument type is void*. + const PointerType *PT = FPT->getParamType(0)->getAs(); + if (!PT) + return; + + if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy) + return; + + // Verify the second argument type is integer. + if (!FPT->getParamType(1)->isIntegralOrUnscopedEnumerationType()) + return; + + // Issue a warning. + PathDiagnosticLocation CELoc = + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), filter.checkName_bzero, + "Use of deprecated function in call to 'bzero()'", + "Security", + "The bzero() function is obsoleted by memset().", + CELoc, CE->getCallee()->getSourceRange()); +} + + //===----------------------------------------------------------------------===// // Check: Any use of 'gets' is insecure. // Originally: @@ -775,6 +913,9 @@ public: checker->filter.checkName_##name = mgr.getCurrentCheckName(); \ } +REGISTER_CHECKER(bcmp) +REGISTER_CHECKER(bcopy) +REGISTER_CHECKER(bzero) REGISTER_CHECKER(gets) REGISTER_CHECKER(getpw) REGISTER_CHECKER(mkstemp) diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m index 8560ba29e7..2c569727ad 100644 --- a/test/Analysis/security-syntax-checks.m +++ b/test/Analysis/security-syntax-checks.m @@ -37,6 +37,27 @@ void test_float_condition() { for (FooType x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}} } +// Obsolete function bcmp +int bcmp(void *, void *, size_t); + +int test_bcmp(void *a, void *b, size_t n) { + return bcmp(a, b, n); // expected-warning{{The bcmp() function is obsoleted by memcmp()}} +} + +// Obsolete function bcopy +void bcopy(void *, void *, size_t); + +void test_bcopy(void *a, void *b, size_t n) { + bcopy(a, b, n); // expected-warning{{The bcopy() function is obsoleted by memcpy() or memmove(}} +} + +// Obsolete function bzero +void bzero(void *, size_t); + +void test_bzero(void *a, size_t n) { + bzero(a, n); // expected-warning{{The bzero() function is obsoleted by memset()}} +} + // rule request: gets() buffer overflow // Part of recommendation: 300-BSI (buildsecurityin.us-cert.gov) char* gets(char *buf); diff --git a/www/analyzer/available_checks.html b/www/analyzer/available_checks.html index b9c7846aa2..f764e0a197 100644 --- a/www/analyzer/available_checks.html +++ b/www/analyzer/available_checks.html @@ -1172,6 +1172,40 @@ void test() { + + + + + + + + + +