Skip to content

[-Wunsafe-buffer-usage] Reduce false positives with constant arrays in libc warnings #108308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions clang/lib/Analysis/UnsafeBufferUsage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -833,9 +833,16 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
//
// For the first two arguments: `ptr` and `size`, they are safe if in the
// following patterns:
//
// Pattern 1:
// ptr := DRE.data();
// size:= DRE.size()/DRE.size_bytes()
// And DRE is a hardened container or view.
//
// Pattern 2:
// ptr := Constant-Array-DRE;
// size:= any expression that has compile-time constant value equivalent to
// sizeof (Constant-Array-DRE)
AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
const FunctionDecl *FD = Node.getDirectCallee();

Expand All @@ -856,6 +863,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
!Size->getType()->isIntegerType())
return false; // not an snprintf call

// Pattern 1:
static StringRef SizedObjs[] = {"span", "array", "vector",
"basic_string_view", "basic_string"};
Buf = Buf->IgnoreParenImpCasts();
Expand Down Expand Up @@ -886,6 +894,23 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
SizedObj)
return false; // It is in fact safe
}

// Pattern 2:
if (auto *DRE = dyn_cast<DeclRefExpr>(Buf->IgnoreParenImpCasts())) {
ASTContext &Ctx = Finder->getASTContext();

if (auto *CAT = Ctx.getAsConstantArrayType(DRE->getType())) {
Expr::EvalResult ER;
// The array element type must be compatible with `char` otherwise an
// explicit cast will be needed, which will make this check unreachable.
// Therefore, the array extent is same as its' bytewise size.
if (Size->EvaluateAsConstantExpr(ER, Ctx)) {
APSInt EVal = ER.Val.getInt(); // Size must have integer type

return APSInt::compareValues(EVal, APSInt(CAT->getSize(), true)) != 0;
}
}
}
return true; // ptr and size are not in safe pattern
}
} // namespace libc_func_matchers
Expand Down
14 changes: 13 additions & 1 deletion clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
sscanf(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf' is unsafe}}
sscanf_s(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf_s' is unsafe}}
fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, p); // expected-warning{{function 'fprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}

char a[10], b[11];
int c[10];

snprintf(a, sizeof(b), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
printf("%s%d", "hello", *p); // no warn
snprintf(s.data(), s.size_bytes(), "%s%d", "hello", *p); // no warn
Expand All @@ -93,13 +99,19 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
__asan_printf();// a printf but no argument, so no warn
}

void v(std::string s1, int *p) {
void safe_examples(std::string s1, int *p) {
snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
fprintf((FILE*)0, s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn

char a[10];

snprintf(a, sizeof a, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
snprintf(a, sizeof(decltype(a)), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
snprintf(a, 10, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
}


Expand Down
Loading