Author: devnexen Date: Thu Jul 19 14:50:03 2018 New Revision: 337499 URL: http://llvm.org/viewvc/llvm-project?rev=337499&view=rev Log: [CStringSyntaxChecker] Check strlcpy sizeof syntax
The last argument is expected to be the destination buffer size (or less). Detects if it points to destination buffer size directly or via a variable. Detects if it is an integral, try to detect if the destination buffer can receive the source length. Updating bsd-string.c unit tests as it make it fails now. Reviewers: george.karpenpov, NoQ Reviewed By: george.karpenkov Differential Revision: https://reviews.llvm.org/D48884 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp cfe/trunk/test/Analysis/bsd-string.c cfe/trunk/test/Analysis/cstring-syntax.c Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp?rev=337499&r1=337498&r2=337499&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp Thu Jul 19 14:50:03 2018 @@ -80,6 +80,17 @@ class WalkAST: public StmtVisitor<WalkAS /// 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, "abcd", cpy); + bool containsBadStrlcpyPattern(const CallExpr *CE); + public: WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC) : Checker(Checker), BR(BR), AC(AC) {} @@ -130,6 +141,38 @@ bool WalkAST::containsBadStrncatPattern( 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->IgnoreParenCasts()); + const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts()); + // - 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->IgnoreParenCasts())) { + uint64_t ILRawVal = IL->getValue().getZExtValue(); + if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) { + ASTContext &C = BR.getContext(); + uint64_t Usize = C.getTypeSizeInChars(DstArg->getType()).getQuantity(); + uint64_t BufferLen = BR.getContext().getTypeSize(Buffer) / Usize; + if (BufferLen < ILRawVal) + return true; + } + } + + return false; +} + void WalkAST::VisitCallExpr(CallExpr *CE) { const FunctionDecl *FD = CE->getDirectCallee(); if (!FD) @@ -157,6 +200,25 @@ void WalkAST::VisitCallExpr(CallExpr *CE BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument", "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()); } } Modified: cfe/trunk/test/Analysis/bsd-string.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bsd-string.c?rev=337499&r1=337498&r2=337499&view=diff ============================================================================== --- cfe/trunk/test/Analysis/bsd-string.c (original) +++ cfe/trunk/test/Analysis/bsd-string.c Thu Jul 19 14:50:03 2018 @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring.NullArg,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s #define NULL ((void *)0) Modified: cfe/trunk/test/Analysis/cstring-syntax.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=337499&r1=337498&r2=337499&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cstring-syntax.c (original) +++ cfe/trunk/test/Analysis/cstring-syntax.c Thu Jul 19 14:50:03 2018 @@ -3,6 +3,7 @@ typedef __SIZE_TYPE__ size_t; char *strncat(char *, const char *, size_t); size_t strlen (const char *s); +size_t strlcpy(char *, const char *, size_t); void testStrncat(const char *src) { char dest[10]; @@ -13,3 +14,17 @@ void testStrncat(const char *src) { // Should not crash when sizeof has a type argument. strncat(dest, "AAAAAAAAAAAAAAAAAAAAAAAAAAA", sizeof(char)); } + +void testStrlcpy(const char *src) { + char dest[10]; + size_t destlen = sizeof(dest); + size_t srclen = sizeof(src); + size_t badlen = 20; + size_t ulen; + strlcpy(dest, src, sizeof(dest)); + strlcpy(dest, src, destlen); + strlcpy(dest, src, 10); + strlcpy(dest, src, 20); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}} + strlcpy(dest, src, badlen); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}} + strlcpy(dest, src, ulen); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits