r342832 - [CStringSyntaxChecker] Check strlcat sizeof check
David Carlier via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 23 01:30:18 PDT 2018
Author: devnexen
Date: Sun Sep 23 01:30:17 2018
New Revision: 342832
URL: http://llvm.org/viewvc/llvm-project?rev=342832&view=rev
Log:
[CStringSyntaxChecker] Check strlcat sizeof check
Assuming strlcat is used with strlcpy we check as we can if the last argument does not equal os not larger than the buffer.
Advising the proper usual pattern.
Reviewers: george.karpenkov, NoQ, MaskRay
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D49722
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
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=342832&r1=342831&r2=342832&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp Sun Sep 23 01:30:17 2018
@@ -90,7 +90,16 @@ class WalkAST: public StmtVisitor<WalkAS
/// 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 @@ bool WalkAST::containsBadStrncatPattern(
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 @@ bool WalkAST::containsBadStrlcpyPattern(
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 @@ void WalkAST::VisitCallExpr(CallExpr *CE
"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 @@ void WalkAST::VisitCallExpr(CallExpr *CE
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());
}
}
Modified: cfe/trunk/test/Analysis/cstring-syntax.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=342832&r1=342831&r2=342832&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cstring-syntax.c (original)
+++ cfe/trunk/test/Analysis/cstring-syntax.c Sun Sep 23 01:30:17 2018
@@ -7,6 +7,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);
+size_t strlcat(char *, const char *, size_t);
void testStrncat(const char *src) {
char dest[10];
@@ -27,9 +28,27 @@ void testStrlcpy(const char *src) {
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}}
}
More information about the cfe-commits
mailing list