summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp54
-rw-r--r--clang/test/Analysis/cstring-syntax.c17
2 files changed, 4 insertions, 67 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
index 2763408face..8b4aa857e77 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,16 +90,7 @@ class WalkAST: public StmtVisitor<WalkAST> {
/// strlcpy(dst, "abcd", 4);
/// strlcpy(dst + 3, "abcd", 2);
/// strlcpy(dst, "abcd", cpy);
- /// 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);
+ bool containsBadStrlcpyPattern(const CallExpr *CE);
public:
WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
@@ -151,21 +142,15 @@ bool WalkAST::containsBadStrncatPattern(const CallExpr *CE) {
return false;
}
-bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyPattern(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;
- // - sizeof(dst)
- // strlcat appends at most size - strlen(dst) - 1
- if (Append && isSizeof(LenArg, DstArg))
- return true;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl());
@@ -196,10 +181,7 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) {
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
- auto RemainingBufferLen = BufferLen - DstOff;
- if (Append)
- RemainingBufferLen -= 1;
- if (RemainingBufferLen < ILRawVal)
+ if ((BufferLen - DstOff) < ILRawVal)
return true;
}
}
@@ -238,7 +220,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
LenArg->getSourceRange());
}
} else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
- if (containsBadStrlcpyStrlcatPattern(CE)) {
+ if (containsBadStrlcpyPattern(CE)) {
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
PathDiagnosticLocation Loc =
@@ -256,34 +238,6 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
"C String API", os.str(), Loc,
LenArg->getSourceRange());
}
- } else if (CheckerContext::isCLibraryFunction(FD, "strlcat")) {
- if (containsBadStrlcpyStrlcatPattern(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);
- StringRef LenName = getPrintableName(LenArg);
-
- SmallString<256> S;
- llvm::raw_svector_ostream os(S);
- os << "The third argument allows to potentially copy more bytes than it should. ";
- os << "Replace with the value ";
- if (!LenName.empty())
- os << "'" << LenName << "'";
- else
- os << " <size> ";
- if (!DstName.empty())
- os << " - strlen(" << DstName << ")";
- else
- os << " - strlen(<destination buffer>)";
- os << " - 1 or lower";
-
- BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
- "C String API", os.str(), Loc,
- LenArg->getSourceRange());
- }
}
// Recurse and check children.
diff --git a/clang/test/Analysis/cstring-syntax.c b/clang/test/Analysis/cstring-syntax.c
index c6d36940605..fe1253bedba 100644
--- a/clang/test/Analysis/cstring-syntax.c
+++ b/clang/test/Analysis/cstring-syntax.c
@@ -7,7 +7,6 @@ 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];
@@ -34,19 +33,3 @@ void testStrlcpy(const char *src) {
strlcpy(dest + 5, src, 5);
strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
}
-
-void testStrlcat(const char *src) {
- char dest[10];
- size_t badlen = 10;
- 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)); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value <size> - strlen(dest) - 1 or lower}}
- strlcpy(dest, "0123456789", sizeof(dest));
- strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value 'badlen' - strlen(dest) - 1 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 'badlen' - strlen(<destination buffer>) - 1 or lower}}
-}
OpenPOWER on IntegriCloud