hazohelet updated this revision to Diff 552939.
hazohelet marked 4 inline comments as done.
hazohelet added a comment.
Address review comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158562/new/
https://reviews.llvm.org/D158562
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Analysis/taint-generic.c
clang/test/Sema/format-strings.c
clang/test/Sema/warn-fortify-source.c
Index: clang/test/Sema/warn-fortify-source.c
===================================================================
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -2,6 +2,8 @@
// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -Wno-fortify-source %s -verify=nofortify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 -Wno-fortify-source %s -verify=nofortify
typedef unsigned long size_t;
@@ -87,6 +89,10 @@
char buf[10];
__builtin_snprintf(buf, 10, "merp");
__builtin_snprintf(buf, 11, "merp"); // expected-warning {{'snprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
+ __builtin_snprintf(buf, 0, "merp");
+ __builtin_snprintf(buf, 3, "merp"); // expected-warning {{'snprintf' will always be truncated; specified size is 3, but format string expands to at least 5}}
+ __builtin_snprintf(buf, 4, "merp"); // expected-warning {{'snprintf' will always be truncated; specified size is 4, but format string expands to at least 5}}
+ __builtin_snprintf(buf, 5, "merp");
}
void call_vsnprintf(void) {
@@ -94,14 +100,20 @@
__builtin_va_list list;
__builtin_vsnprintf(buf, 10, "merp", list);
__builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
+ __builtin_vsnprintf(buf, 0, "merp", list);
+ __builtin_vsnprintf(buf, 3, "merp", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 3, but format string expands to at least 5}}
+ __builtin_vsnprintf(buf, 4, "merp", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 4, but format string expands to at least 5}}
+ __builtin_vsnprintf(buf, 5, "merp", list);
}
void call_sprintf_chk(char *buf) {
__builtin___sprintf_chk(buf, 1, 6, "hell\n");
__builtin___sprintf_chk(buf, 1, 5, "hell\n"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
- __builtin___sprintf_chk(buf, 1, 6, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
- __builtin___sprintf_chk(buf, 1, 2, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
- // expected-warning@-1 {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 5}}
+ __builtin___sprintf_chk(buf, 1, 6, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+ // nofortify-warning {{format string contains '\0' within the string body}}
+ __builtin___sprintf_chk(buf, 1, 2, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+ // nofortify-warning {{format string contains '\0' within the string body}}
+ // expected-warning@-2 {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 5}}
__builtin___sprintf_chk(buf, 1, 6, "hello");
__builtin___sprintf_chk(buf, 1, 5, "hello"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
__builtin___sprintf_chk(buf, 1, 2, "%c", '9');
@@ -150,9 +162,11 @@
void call_sprintf(void) {
char buf[6];
- sprintf(buf, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
- sprintf(buf, "hello b\0y"); // expected-warning {{format string contains '\0' within the string body}}
- // expected-warning@-1 {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 8}}
+ sprintf(buf, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+ // nofortify-warning {{format string contains '\0' within the string body}}
+ sprintf(buf, "hello b\0y"); // expected-warning {{format string contains '\0' within the string body}} \
+ // nofortify-warning {{format string contains '\0' within the string body}}
+ // expected-warning@-2 {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 8}}
sprintf(buf, "hello");
sprintf(buf, "hello!"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
sprintf(buf, "1234%%");
Index: clang/test/Sema/format-strings.c
===================================================================
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -202,7 +202,8 @@
printf("%s%lv%d","unix",10,20); // expected-warning {{invalid conversion specifier 'v'}} expected-warning {{data argument not used by format string}}
fprintf(fp,"%%%l"); // expected-warning {{incomplete format specifier}}
sprintf(buf,"%%%%%ld%d%d", 1, 2, 3); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
- snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning{{format specifies type 'long' but the argument has type 'int'}} expected-warning {{invalid conversion specifier ';'}} expected-warning {{data argument not used by format string}}
+ snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning{{format specifies type 'long' but the argument has type 'int'}} expected-warning {{invalid conversion specifier ';'}} expected-warning {{data argument not used by format string}} \
+ // expected-warning{{'snprintf' will always be truncated; specified size is 2, but format string expands to at least 7}}
}
void check_null_char_string(char* b)
Index: clang/test/Analysis/taint-generic.c
===================================================================
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -229,6 +229,7 @@
char addr[128];
scanf("%s", addr);
__builtin_snprintf(buffern, 10, "/bin/mail %s < /tmp/email", addr);
+ // expected-warning@-1 {{'snprintf' will always be truncated; specified size is 10, but format string expands to at least 24}}
system(buffern); // expected-warning {{Untrusted data is passed to a system call}}
}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1032,6 +1032,23 @@
} // namespace
+static bool ProcessFormatStringLiteral(const Expr *FormatExpr,
+ StringRef &FormatStrRef, size_t &StrLen,
+ ASTContext &Context) {
+ if (auto *Format = dyn_cast<StringLiteral>(FormatExpr);
+ Format && (Format->isOrdinary() || Format->isUTF8())) {
+ FormatStrRef = Format->getString();
+ const ConstantArrayType *T =
+ Context.getAsConstantArrayType(Format->getType());
+ assert(T && "String literal not of constant array type!");
+ size_t TypeSize = T->getSize().getZExtValue();
+ // In case there's a null byte somewhere.
+ StrLen = std::min(std::max(TypeSize, size_t(1)) - 1, FormatStrRef.find(0));
+ return true;
+ }
+ return false;
+}
+
void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
CallExpr *TheCall) {
if (TheCall->isValueDependent() || TheCall->isTypeDependent() ||
@@ -1183,11 +1200,9 @@
const auto *FormatExpr =
TheCall->getArg(FormatIndex)->IgnoreParenImpCasts();
- const auto *Format = dyn_cast<StringLiteral>(FormatExpr);
- if (!Format)
- return;
-
- if (!Format->isOrdinary() && !Format->isUTF8())
+ StringRef FormatStrRef;
+ size_t StrLen;
+ if (!ProcessFormatStringLiteral(FormatExpr, FormatStrRef, StrLen, Context))
return;
auto Diagnose = [&](unsigned ArgIndex, unsigned DestSize,
@@ -1200,21 +1215,11 @@
<< DestSize << SourceSize);
};
- StringRef FormatStrRef = Format->getString();
auto ShiftedComputeSizeArgument = [&](unsigned Index) {
return ComputeSizeArgument(Index + DataIndex);
};
ScanfDiagnosticFormatHandler H(ShiftedComputeSizeArgument, Diagnose);
const char *FormatBytes = FormatStrRef.data();
- const ConstantArrayType *T =
- Context.getAsConstantArrayType(Format->getType());
- assert(T && "String literal not of constant array type!");
- size_t TypeSize = T->getSize().getZExtValue();
-
- // In case there's a null byte somewhere.
- size_t StrLen =
- std::min(std::max(TypeSize, size_t(1)) - 1, FormatStrRef.find(0));
-
analyze_format_string::ParseScanfString(H, FormatBytes,
FormatBytes + StrLen, getLangOpts(),
Context.getTargetInfo());
@@ -1230,22 +1235,11 @@
size_t FormatIndex = BuiltinID == Builtin::BIsprintf ? 1 : 3;
auto *FormatExpr = TheCall->getArg(FormatIndex)->IgnoreParenImpCasts();
- if (auto *Format = dyn_cast<StringLiteral>(FormatExpr)) {
-
- if (!Format->isOrdinary() && !Format->isUTF8())
- return;
-
- StringRef FormatStrRef = Format->getString();
+ StringRef FormatStrRef;
+ size_t StrLen;
+ if (ProcessFormatStringLiteral(FormatExpr, FormatStrRef, StrLen, Context)) {
EstimateSizeFormatHandler H(FormatStrRef);
const char *FormatBytes = FormatStrRef.data();
- const ConstantArrayType *T =
- Context.getAsConstantArrayType(Format->getType());
- assert(T && "String literal not of constant array type!");
- size_t TypeSize = T->getSize().getZExtValue();
-
- // In case there's a null byte somewhere.
- size_t StrLen =
- std::min(std::max(TypeSize, size_t(1)) - 1, FormatStrRef.find(0));
if (!analyze_format_string::ParsePrintfString(
H, FormatBytes, FormatBytes + StrLen, getLangOpts(),
Context.getTargetInfo(), false)) {
@@ -1326,8 +1320,28 @@
case Builtin::BI__builtin_vsnprintf: {
DiagID = diag::warn_fortify_source_size_mismatch;
SourceSize = ComputeExplicitObjectSizeArgument(1);
+ const auto *FormatExpr = TheCall->getArg(2)->IgnoreParenImpCasts();
+ StringRef FormatStrRef;
+ size_t StrLen;
+ if (SourceSize &&
+ ProcessFormatStringLiteral(FormatExpr, FormatStrRef, StrLen, Context)) {
+ EstimateSizeFormatHandler H(FormatStrRef);
+ const char *FormatBytes = FormatStrRef.data();
+ if (!analyze_format_string::ParsePrintfString(
+ H, FormatBytes, FormatBytes + StrLen, getLangOpts(),
+ Context.getTargetInfo(), /*isFreeBSDKPrintf=*/false)) {
+ llvm::APSInt FormatSize =
+ llvm::APSInt::getUnsigned(H.getSizeLowerBound())
+ .extOrTrunc(SizeTypeWidth);
+ if (FormatSize > *SourceSize && *SourceSize != 0) {
+ DiagID = diag::warn_fortify_source_format_truncation;
+ DestinationSize = SourceSize;
+ SourceSize = FormatSize;
+ break;
+ }
+ }
+ }
DestinationSize = ComputeSizeArgument(0);
- break;
}
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -857,6 +857,11 @@
" but format string expands to at least %2">,
InGroup<FortifySource>;
+def warn_fortify_source_format_truncation: Warning<
+ "'%0' will always be truncated; specified size is %1,"
+ " but format string expands to at least %2">,
+ InGroup<FortifySource>;
+
def warn_fortify_scanf_overflow : Warning<
"'%0' may overflow; destination buffer in argument %1 has size "
"%2, but the corresponding specifier may require size %3">,
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -140,6 +140,9 @@
tautologies like ``x && !x`` and ``!x || x`` in expressions. This also
makes ``-Winfinite-recursion`` diagnose more cases.
(`#56035: <https://github.com/llvm/llvm-project/issues/56035>`_).
+- Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
+ result in string truncation.
+ (`#64871: <https://github.com/llvm/llvm-project/issues/64871>`_).
Bug Fixes in This Version
-------------------------
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
