Implement: <rdar://problem/6337100> CWE-338: Use of cryptographically weak prng

Patch by Geoff Keating!

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@80752 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/CheckSecuritySyntaxOnly.cpp b/lib/Analysis/CheckSecuritySyntaxOnly.cpp
index 3684360835cb8ab79f055541177ec4713f100e41..1bed9d1a5b87cc0ab0e60e36db6ffac51b9f0a65 100644 (file)
--- a/lib/Analysis/CheckSecuritySyntaxOnly.cpp
+++ b/lib/Analysis/CheckSecuritySyntaxOnly.cpp
@@ -23,12 +23,15 @@ namespace {
 class VISIBILITY_HIDDEN WalkAST : public StmtVisitor<WalkAST> {
   BugReporter &BR;  
   IdentifierInfo *II_gets;
-  enum { num_ids = 6 };
-  IdentifierInfo *II_setid[num_ids];
+  enum { num_rands = 9 };
+  IdentifierInfo *II_rand[num_rands];
+  IdentifierInfo *II_random;
+  enum { num_setids = 6 };
+  IdentifierInfo *II_setid[num_setids];
   
 public:
   WalkAST(BugReporter &br) : BR(br),
-    II_gets(0), II_setid() {}
+    II_gets(0), II_rand(), II_random(0), II_setid() {}
   
   // Statement visitor methods.
   void VisitCallExpr(CallExpr *CE);
@@ -44,6 +47,8 @@ public:
   // Checker-specific methods.
   void CheckLoopConditionForFloat(const ForStmt *FS);
   void CheckCall_gets(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 CheckUncheckedReturnValue(CallExpr *CE);
 };
 } // end anonymous namespace
@@ -71,7 +76,9 @@ void WalkAST::VisitChildren(Stmt *S) {
 
 void WalkAST::VisitCallExpr(CallExpr *CE) {
   if (const FunctionDecl *FD = CE->getDirectCallee()) {
-    CheckCall_gets(CE, FD);    
+    CheckCall_gets(CE, FD);
+    CheckCall_rand(CE, FD);
+    CheckCall_random(CE, FD);
   }
   
   // Recurse and check children.
@@ -240,6 +247,96 @@ void WalkAST::CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
                      CE->getLocStart(), &R, 1);
 }
 
+//===----------------------------------------------------------------------===//
+// Check: Linear congruent random number generators should not be used
+// Originally: <rdar://problem/63371000>
+// CWE-338: Use of cryptographically weak prng
+//===----------------------------------------------------------------------===//
+
+void WalkAST::CheckCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
+  if (II_rand[0] == NULL) {
+    // This check applies to these functions
+    static const char * const identifiers[num_rands] = {
+      "drand48", "erand48", "jrand48", "lrand48", "mrand48", "nrand48",
+      "lcong48",
+      "rand", "rand_r"
+    };
+      
+    for (size_t i = 0; i < num_rands; i++)
+      II_rand[i] = &BR.getContext().Idents.get(identifiers[i]);  
+  }
+  
+  const IdentifierInfo *id = FD->getIdentifier();
+  size_t identifierid;
+
+  for (identifierid = 0; identifierid < num_rands; identifierid++)
+    if (id == II_rand[identifierid])
+      break;
+
+  if (identifierid >= num_rands)
+    return;
+  
+  const FunctionProtoType *FTP = dyn_cast<FunctionProtoType>(FD->getType());
+  if (!FTP)
+    return;
+  
+  if (FTP->getNumArgs() == 1) {
+    // Is the argument an 'unsigned short *'?
+    // (Actually any integer type is allowed.)
+    const PointerType *PT = dyn_cast<PointerType>(FTP->getArgType(0));
+    if (!PT)
+      return;
+      
+    if (! PT->getPointeeType()->isIntegerType())
+      return;
+  }
+  else if (FTP->getNumArgs() != 0) 
+    return;
+  
+  // Issue a warning.
+  std::string buf1;
+  llvm::raw_string_ostream os1(buf1);
+  os1 << "'" << FD->getNameAsString() << "' is a poor random number generator";
+
+  std::string buf2;
+  llvm::raw_string_ostream os2(buf2);
+  os2 << "Function '" << FD->getNameAsString()
+      << "' is obsolete because it implements a poor random number generator."
+      << "  Use 'arc4random' instead";
+
+  SourceRange R = CE->getCallee()->getSourceRange();
+  
+  BR.EmitBasicReport(os1.str().c_str(), "Security", os2.str().c_str(),
+                     CE->getLocStart(), &R, 1);
+}
+
+//===----------------------------------------------------------------------===//
+// Check: 'random' should not be used
+// Originally: <rdar://problem/63371000>
+//===----------------------------------------------------------------------===//
+
+void WalkAST::CheckCall_random(const CallExpr *CE, const FunctionDecl *FD) {
+  if (FD->getIdentifier() != GetIdentifier(II_random, "random"))
+    return;
+  
+  const FunctionProtoType *FTP = dyn_cast<FunctionProtoType>(FD->getType());
+  if (!FTP)
+    return;
+  
+  // Verify that the function takes no argument.
+  if (FTP->getNumArgs() != 0)
+    return;
+  
+  // Issue a warning.
+  SourceRange R = CE->getCallee()->getSourceRange();
+  BR.EmitBasicReport("'random' is not a secure random number generator",
+                     "Security",
+                     "The 'random' function produces a sequence of values that "
+                     "an adversary may be able to predict.  Use 'arc4random' "
+                     "instead",
+                     CE->getLocStart(), &R, 1);
+}
+
 //===----------------------------------------------------------------------===//
 // Check: Should check whether privileges are dropped successfully.
 // Originally: <rdar://problem/6337132>
@@ -251,23 +348,23 @@ void WalkAST::CheckUncheckedReturnValue(CallExpr *CE) {
     return;
 
   if (II_setid[0] == NULL) {
-    static const char * const identifiers[num_ids] = {
+    static const char * const identifiers[num_setids] = {
       "setuid", "setgid", "seteuid", "setegid",
       "setreuid", "setregid"
     };
       
-    for (size_t i = 0; i < num_ids; i++)
+    for (size_t i = 0; i < num_setids; i++)
       II_setid[i] = &BR.getContext().Idents.get(identifiers[i]);  
   }
   
   const IdentifierInfo *id = FD->getIdentifier();
   size_t identifierid;
 
-  for (identifierid = 0; identifierid < num_ids; identifierid++)
+  for (identifierid = 0; identifierid < num_setids; identifierid++)
     if (id == II_setid[identifierid])
       break;
 
-  if (identifierid >= num_ids)
+  if (identifierid >= num_setids)
     return;
   
   const FunctionProtoType *FTP = dyn_cast<FunctionProtoType>(FD->getType());

diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m
index df17926a15e031f2d38a223010e591b905a7a728..ebd7d172b5b4794c0ea8ae28543c0160aa5dc687 100644 (file)
--- a/test/Analysis/security-syntax-checks.m
+++ b/test/Analysis/security-syntax-checks.m
@@ -60,3 +60,32 @@ void test_setuid()
   setreuid(2,2); // expected-warning{{The return value from the call to 'setreuid' is not checked.  If an error occurs in 'setreuid', the following code may execute with unexpected privileges}}
   setregid(2,2); // expected-warning{{The return value from the call to 'setregid' is not checked.  If an error occurs in 'setregid', the following code may execute with unexpected privileges}}
 }
+
+// <rdar://problem/6337100> CWE-338: Use of cryptographically weak prng
+int      rand(void);
+double   drand48(void);
+double   erand48(unsigned short[3]);
+long     jrand48(unsigned short[3]);
+void     lcong48(unsigned short[7]);
+long     lrand48(void);
+long     mrand48(void);
+long     nrand48(unsigned short[3]);
+long     random(void);
+int      rand_r(unsigned *);
+
+void test_rand()
+{
+  unsigned short a[7];
+  unsigned b;
+  
+  rand();      // expected-warning{{Function 'rand' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  drand48();   // expected-warning{{Function 'drand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  erand48(a);  // expected-warning{{Function 'erand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  jrand48(a);  // expected-warning{{Function 'jrand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  lcong48(a);  // expected-warning{{Function 'lcong48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  lrand48();   // expected-warning{{Function 'lrand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  mrand48();   // expected-warning{{Function 'mrand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  nrand48(a);  // expected-warning{{Function 'nrand48' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  rand_r(&b);  // expected-warning{{Function 'rand_r' is obsolete because it implements a poor random number generator.  Use 'arc4random' instead}}
+  random();    // expected-warning{{The 'random' function produces a sequence of values that an adversary may be able to predict.  Use 'arc4random' instead}}
+}