Implement: <rdar://problem/6335715> rule request: gets() buffer overflow

llvm-svn: 76905

clang/lib/Analysis/CheckSecuritySyntaxOnly.cpp

@@ -21,21 +21,39 @@ using namespace clang;
 
 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 @@ 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: <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.
 //===----------------------------------------------------------------------===//

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'}}
 }
 
+// <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}}
+}