devnexen updated this revision to Diff 166628.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===================================================================
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -7,6 +7,7 @@
char *strncat(char *, const char *, size_t);
size_t strlen (const char *s);
size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
void testStrncat(const char *src) {
char dest[10];
@@ -27,9 +28,27 @@
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, 20); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcpy(dest, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
strlcpy(dest, src, ulen);
strlcpy(dest + 5, src, 5);
- strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
+ strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(<destination buffer>) or lower}}
+}
+
+void testStrlcat(const char *src) {
+ char dest[10];
+ size_t badlen = 20;
+ size_t ulen;
+ strlcpy(dest, "aaaaa", sizeof("aaaaa") - 1);
+ strlcat(dest, "bbbb", (sizeof("bbbb") - 1) - sizeof(dest) - 1);
+ strlcpy(dest, "012345678", sizeof(dest));
+ strlcat(dest, "910", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
+ strlcat(dest, src, ulen);
+ strlcpy(dest, src, 5);
+ strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(<destination buffer>) or lower}}
}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,7 +90,16 @@
/// strlcpy(dst, "abcd", 4);
/// strlcpy(dst + 3, "abcd", 2);
/// strlcpy(dst, "abcd", cpy);
- bool containsBadStrlcpyPattern(const CallExpr *CE);
+ /// Identify erroneous patterns in the last argument to strlcat - the number
+ /// of bytes to copy.
+ /// The bad pattern checked is when the last argument is basically
+ /// pointing to the destination buffer size or argument larger or
+ /// equal to.
+ /// char dst[2];
+ /// strlcat(dst, src2, sizeof(dst));
+ /// strlcat(dst, src2, 2);
+ /// strlcat(dst, src2, 10);
+ bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE);
public:
WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
@@ -142,15 +151,19 @@
return false;
}
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
if (CE->getNumArgs() != 3)
return false;
+ const FunctionDecl *FD = CE->getDirectCallee();
+ bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
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;
+ if (isSizeof(LenArg, DstArg))
+ return false;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@
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;
+ auto RemainingBufferLen = BufferLen - DstOff;
+ if (Append) {
+ if (RemainingBufferLen <= ILRawVal)
+ return true;
+ } else {
+ if (RemainingBufferLen < ILRawVal)
+ return true;
+ }
}
}
}
@@ -219,8 +238,9 @@
"C String API", os.str(), Loc,
LenArg->getSourceRange());
}
- } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
- if (containsBadStrlcpyPattern(CE)) {
+ } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy") ||
+ CheckerContext::isCLibraryFunction(FD, "strlcat")) {
+ if (containsBadStrlcpyStrlcatPattern(CE)) {
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
PathDiagnosticLocation Loc =
@@ -230,13 +250,17 @@
SmallString<256> S;
llvm::raw_svector_ostream os(S);
- os << "The third argument is larger than the size of the input buffer. ";
+ os << "The third argument allows to potentially copy more bytes than it should. ";
+ os << "Replace with the value ";
if (!DstName.empty())
- os << "Replace with the value 'sizeof(" << DstName << ")` or lower";
+ os << "sizeof(" << DstName << ")";
+ else
+ os << "sizeof(<destination buffer>)";
+ os << " or lower";
BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
- "C String API", os.str(), Loc,
- LenArg->getSourceRange());
+ "C String API", os.str(), Loc,
+ LenArg->getSourceRange());
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
