[PATCH] D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error.
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 8 15:01:59 PST 2019
NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, devnexen.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.
That's a fix on top of D49722 <https://reviews.llvm.org/D49722>.
Both `strlcat` and `strlcpy` cut off their safe bound argument value at `sizeof(destination)`. There's no need to subtract 1 in one of these cases.
Repository:
rC Clang
https://reviews.llvm.org/D57981
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
@@ -33,6 +33,7 @@
strlcpy(dest, src, ulen);
strlcpy(dest + 5, src, 5);
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}}
+ strlcpy(dest, "aaaaaaaaaaaaaaa", 10); // no-warning
}
void testStrlcat(const char *src) {
@@ -51,4 +52,5 @@
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}}
+ strlcat(dest, "aaaaaaaaaaaaaaa", 10); // no-warning
}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -153,8 +153,6 @@
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);
@@ -194,13 +192,8 @@
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
auto RemainingBufferLen = BufferLen - DstOff;
- if (Append) {
- if (RemainingBufferLen <= ILRawVal)
- return true;
- } else {
- if (RemainingBufferLen < ILRawVal)
- return true;
- }
+ if (RemainingBufferLen < ILRawVal)
+ return true;
}
}
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57981.186049.patch
Type: text/x-patch
Size: 1904 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190208/bb553e02/attachment.bin>
More information about the cfe-commits
mailing list