[cfe-commits] r76905 - in /cfe/trunk: lib/Analysis/CheckSecuritySyntaxOnly.cpp test/Analysis/security-syntax-checks.m

[Ted Kremenek]

Author: kremenek
Date: Thu Jul 23 17:29:41 2009
New Revision: 76905

URL: http://llvm.org/viewvc/llvm-project?rev=76905&view=rev
Log:
Implement: <rdar://problem/6335715> rule request: gets() buffer overflow

Modified:
    cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
    cfe/trunk/test/Analysis/security-syntax-checks.m

Modified: cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp?rev=76905&r1=76904&r2=76905&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp Thu Jul 23 17:29:41 2009
@@ -21,22 +21,40 @@
 
 namespace {
 class VISIBILITY_HIDDEN WalkAST : public StmtVisitor<WalkAST> {
-  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 @@
       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);  
 
@@ -162,6 +189,41 @@
 }
 
 //===----------------------------------------------------------------------===//
+// Check: Any use of 'gets' is insecure.
+// Originally: <rdar://problem/6335715>
+// 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<FunctionProtoType>(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<PointerType>(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.
 //===----------------------------------------------------------------------===//
 

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=76905&r1=76904&r2=76905&view=diff

==============================================================================
--- cfe/trunk/test/Analysis/security-syntax-checks.m (original)
+++ cfe/trunk/test/Analysis/security-syntax-checks.m Thu Jul 23 17:29:41 2009
@@ -21,3 +21,11 @@
   for (FooType x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}}
 }
 
+// <rdar://problem/6335715> 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}}
+}