diff --git a/clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp b/clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp index 36b729e21959..5c98a526b7f6 100644 --- a/clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp @@ -21,21 +21,39 @@ using namespace clang; namespace { class VISIBILITY_HIDDEN WalkAST : public StmtVisitor { - BugReporter &BR; + BugReporter &BR; + IdentifierInfo *II_gets; public: - WalkAST(BugReporter &br) : BR(br) {} + WalkAST(BugReporter &br) : BR(br), + II_gets(0) {} // Statement visitor methods. + void VisitCallExpr(CallExpr *CE); void VisitForStmt(ForStmt *S); void VisitStmt(Stmt *S) { VisitChildren(S); } void VisitChildren(Stmt *S); + // Helpers. + IdentifierInfo *GetIdentifier(IdentifierInfo *& II, const char *str); + // Checker-specific methods. void CheckLoopConditionForFloat(const ForStmt *FS); + void CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD); }; } // end anonymous namespace +//===----------------------------------------------------------------------===// +// Helper methods. +//===----------------------------------------------------------------------===// + +IdentifierInfo *WalkAST::GetIdentifier(IdentifierInfo *& II, const char *str) { + if (!II) + II = &BR.getContext().Idents.get(str); + + return II; +} + //===----------------------------------------------------------------------===// // AST walking. //===----------------------------------------------------------------------===// @@ -46,6 +64,15 @@ void WalkAST::VisitChildren(Stmt *S) { Visit(child); } +void WalkAST::VisitCallExpr(CallExpr *CE) { + if (const FunctionDecl *FD = CE->getDirectCallee()) { + CheckCall_gets(CE, FD); + } + + // Recurse and check children. + VisitChildren(CE); +} + void WalkAST::VisitForStmt(ForStmt *FS) { CheckLoopConditionForFloat(FS); @@ -161,6 +188,41 @@ void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) { FS->getLocStart(), ranges.data(), ranges.size()); } +//===----------------------------------------------------------------------===// +// Check: Any use of 'gets' is insecure. +// Originally: +// Implements (part of): 300-BSI (buildsecurityin.us-cert.gov) +//===----------------------------------------------------------------------===// + +void WalkAST::CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD) { + if (FD->getIdentifier() != GetIdentifier(II_gets, "gets")) + return; + + const FunctionProtoType *FTP = dyn_cast(FD->getType()); + if (!FTP) + return; + + // Verify that the function takes a single argument. + if (FTP->getNumArgs() != 1) + return; + + // Is the argument a 'char*'? + const PointerType *PT = dyn_cast(FTP->getArgType(0)); + if (!PT) + return; + + if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy) + return; + + // Issue a warning. + SourceRange R = CE->getCallee()->getSourceRange(); + BR.EmitBasicReport("Potential buffer overflow in call to 'gets'", + "Security", + "Call to function 'gets' is extremely insecure as it can " + "always result in a buffer overflow", + CE->getLocStart(), &R, 1); +} + //===----------------------------------------------------------------------===// // Entry point for check. //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/security-syntax-checks.m b/clang/test/Analysis/security-syntax-checks.m index 897c925e2794..7e2a03d81cb5 100644 --- a/clang/test/Analysis/security-syntax-checks.m +++ b/clang/test/Analysis/security-syntax-checks.m @@ -21,3 +21,11 @@ void test_float_condition() { for (FooType x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}} } +// rule request: gets() buffer overflow +// Part of recommendation: 300-BSI (buildsecurityin.us-cert.gov) +char* gets(char *buf); + +void test_gets() { + char buff[1024]; + gets(buff); // expected-warning{{Call to function 'gets' is extremely insecure as it can always result in a buffer overflow}} +}