r337499 - [CStringSyntaxChecker] Check strlcpy sizeof syntax
David Carlier via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 19 14:50:04 PDT 2018
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);
+}
More information about the cfe-commits
mailing list