Skip to content

Commit

Permalink
[analyzer] CStringSyntaxChecks: Fix an off-by-one error in the strlca…
Browse files Browse the repository at this point in the history
…t() check.

oth strlcat and strlcpy cut off their safe bound for the argument value
at sizeof(destination). There's no need to subtract 1 in only one
of these cases.

Differential Revision: https://reviews.llvm.org/D57981

rdar://problem/47873212

llvm-svn: 353583
  • Loading branch information
haoNoQ committed Feb 8, 2019
1 parent afd612e commit 9197056
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 9 deletions.
11 changes: 2 additions & 9 deletions clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
Expand Up @@ -153,8 +153,6 @@ bool WalkAST::containsBadStrncatPattern(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);

Expand Down Expand Up @@ -194,13 +192,8 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
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;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Analysis/cstring-syntax.c
Expand Up @@ -33,6 +33,7 @@ void testStrlcpy(const char *src) {
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) {
Expand All @@ -51,4 +52,5 @@ void testStrlcat(const char *src) {
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
}

0 comments on commit 9197056

Please sign in to comment.