1 files changed, 79 insertions, 0 deletions

diff --git a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
index 4b5e97b69295a..8b4aa857e7752 100644
--- a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -80,6 +80,18 @@ class WalkAST: public StmtVisitor<WalkAST> {
   /// of bytes to copy.
   bool containsBadStrncatPattern(const CallExpr *CE);
 
+  /// Identify erroneous patterns in the last argument to strlcpy - the number
+  /// of bytes to copy.
+  /// The bad pattern checked is when the size is known
+  /// to be larger than the destination can handle.
+  ///   char dst[2];
+  ///   size_t cpy = 4;
+  ///   strlcpy(dst, "abcd", sizeof("abcd") - 1);
+  ///   strlcpy(dst, "abcd", 4);
+  ///   strlcpy(dst + 3, "abcd", 2);
+  ///   strlcpy(dst, "abcd", cpy);
+  bool containsBadStrlcpyPattern(const CallExpr *CE);
+
 public:
   WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
       : Checker(Checker), BR(BR), AC(AC) {}
@@ -130,6 +142,54 @@ bool WalkAST::containsBadStrncatPattern(const CallExpr *CE) {
   return false;
 }
 
+bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+  if (CE->getNumArgs() != 3)
+    return false;
+  const Expr *DstArg = CE->getArg(0);
+  const Expr *LenArg = CE->getArg(2);
+
+  const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
+  const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
+  uint64_t DstOff = 0;
+  // - size_t dstlen = sizeof(dst)
+  if (LenArgDecl) {
+    const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl());
+    if (LenArgVal->getInit())
+      LenArg = LenArgVal->getInit();
+  }
+
+  // - integral value
+  // We try to figure out if the last argument is possibly longer
+  // than the destination can possibly handle if its size can be defined.
+  if (const auto *IL = dyn_cast<IntegerLiteral>(LenArg->IgnoreParenImpCasts())) {
+    uint64_t ILRawVal = IL->getValue().getZExtValue();
+
+    // Case when there is pointer arithmetic on the destination buffer
+    // especially when we offset from the base decreasing the
+    // buffer length accordingly.
+    if (!DstArgDecl) {
+      if (const auto *BE = dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) {
+        DstArgDecl = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts());
+        if (BE->getOpcode() == BO_Add) {
+          if ((IL = dyn_cast<IntegerLiteral>(BE->getRHS()->IgnoreParenImpCasts()))) {
+            DstOff = IL->getValue().getZExtValue();
+          }
+        }
+      }
+    }
+    if (DstArgDecl) {
+      if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) {
+        ASTContext &C = BR.getContext();
+        uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
+        if ((BufferLen - DstOff) < ILRawVal)
+          return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 void WalkAST::VisitCallExpr(CallExpr *CE) {
   const FunctionDecl *FD = CE->getDirectCallee();
   if (!FD)
@@ -159,6 +219,25 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
                          "C String API", os.str(), Loc,
                          LenArg->getSourceRange());
     }
+  } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
+    if (containsBadStrlcpyPattern(CE)) {
+      const Expr *DstArg = CE->getArg(0);
+      const Expr *LenArg = CE->getArg(2);
+      PathDiagnosticLocation Loc =
+        PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC);
+
+      StringRef DstName = getPrintableName(DstArg);
+
+      SmallString<256> S;
+      llvm::raw_svector_ostream os(S);
+      os << "The third argument is larger than the size of the input buffer. ";
+      if (!DstName.empty())
+        os << "Replace with the value 'sizeof(" << DstName << ")` or lower";
+
+      BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
+                         "C String API", os.str(), Loc,
+                         LenArg->getSourceRange());
+    }
   }
 
   // Recurse and check children.