maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Revert "[clang] Fortify warning for scanf calls with field width too big." 

This reverts commit 5a8c173628.

The warning needs to correctly handle * specifiers (which are to be
ignored).
This commit is contained in:
Michael Benfield 2021-11-01 19:36:45 +00:00
parent 8a992b20db
commit d51a8296d3
3 changed files with 12 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@ -833,10 +833,6 @@ def warn_fortify_source_format_overflow : Warning<
" 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">,
InGroup<FortifySource>;
/// main()
// static main() is not an error in C, just in C++.

143
clang/lib/Sema/SemaChecking.cpp
View File

@ -408,61 +408,6 @@ static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
namespace {
class ScanfDiagnosticFormatHandler
: public analyze_format_string::FormatStringHandler {
// Accepts the argument index (relative to the first destination index) of the
// argument whose size we want.
using ComputeSizeFunction =
llvm::function_ref<Optional<llvm::APSInt>(unsigned)>;
// Accepts the argument index (relative to the first destination index), the
// destination size, and the source size).
using DiagnoseFunction =
llvm::function_ref<void(unsigned, unsigned, unsigned)>;
ComputeSizeFunction ComputeSizeArgument;
DiagnoseFunction Diagnose;
public:
ScanfDiagnosticFormatHandler(ComputeSizeFunction ComputeSizeArgument,
DiagnoseFunction Diagnose)
: ComputeSizeArgument(ComputeSizeArgument), Diagnose(Diagnose) {}
bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
const char *StartSpecifier,
unsigned specifierLen) override {
unsigned NulByte = 0;
switch ((FS.getConversionSpecifier().getKind())) {
default:
return true;
case analyze_format_string::ConversionSpecifier::sArg:
case analyze_format_string::ConversionSpecifier::ScanListArg:
NulByte = 1;
break;
case analyze_format_string::ConversionSpecifier::cArg:
break;
}
auto OptionalFW = FS.getFieldWidth();
if (OptionalFW.getHowSpecified() !=
analyze_format_string::OptionalAmount::HowSpecified::Constant)
return true;
unsigned SourceSize = OptionalFW.getConstantAmount() + NulByte;
auto DestSizeAPS = ComputeSizeArgument(FS.getArgIndex());
if (!DestSizeAPS)
return true;
unsigned DestSize = DestSizeAPS->getZExtValue();
if (DestSize < SourceSize)
Diagnose(FS.getArgIndex(), DestSize, SourceSize);
return true;
}
};
class EstimateSizeFormatHandler
: public analyze_format_string::FormatStringHandler {
size_t Size;
@ -670,12 +615,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
// (potentially) more strict checking mode. Otherwise, conservatively assume
// type 0.
int BOSType = 0;
// This check can fail for variadic functions.
if (Index < FD->getNumParams()) {
if (const auto *POS =
FD->getParamDecl(Index)->getAttr<PassObjectSizeAttr>())
BOSType = POS->getType();
}
if (const auto *POS =
FD->getParamDecl(Index)->getAttr<PassObjectSizeAttr>())
BOSType = POS->getType();
const Expr *ObjArg = TheCall->getArg(Index);
uint64_t Result;
@ -700,20 +642,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
unsigned DiagID = 0;
bool IsChkVariant = false;
auto GetFunctionName = [&]() {
StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
// Skim off the details of whichever builtin was called to produce a better
// diagnostic, as it's unlikely that the user wrote the __builtin
// explicitly.
if (IsChkVariant) {
FunctionName = FunctionName.drop_front(std::strlen("__builtin___"));
FunctionName = FunctionName.drop_back(std::strlen("_chk"));
} else if (FunctionName.startswith("__builtin_")) {
FunctionName = FunctionName.drop_front(std::strlen("__builtin_"));
}
return FunctionName;
};
switch (BuiltinID) {
default:
return;
@ -733,61 +661,6 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
break;
}
case Builtin::BIscanf:
case Builtin::BIfscanf:
case Builtin::BIsscanf: {
unsigned FormatIndex = 1;
unsigned DataIndex = 2;
if (BuiltinID == Builtin::BIscanf) {
FormatIndex = 0;
DataIndex = 1;
}
const auto *FormatExpr =
TheCall->getArg(FormatIndex)->IgnoreParenImpCasts();
const auto *Format = dyn_cast<StringLiteral>(FormatExpr);
if (!Format)
return;
if (!Format->isAscii() && !Format->isUTF8())
return;
auto Diagnose = [&](unsigned ArgIndex, unsigned DestSize,
unsigned SourceSize) {
DiagID = diag::warn_fortify_scanf_overflow;
unsigned Index = ArgIndex + DataIndex;
StringRef FunctionName = GetFunctionName();
DiagRuntimeBehavior(TheCall->getArg(Index)->getBeginLoc(), TheCall,
PDiag(DiagID) << FunctionName << (Index + 1)
<< 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());
// Unlike the other cases, in this one we have already issued the diagnostic
// here, so no need to continue (because unlike the other cases, here the
// diagnostic refers to the argument number).
return;
}
case Builtin::BIsprintf:
case Builtin::BI__builtin___sprintf_chk: {
size_t FormatIndex = BuiltinID == Builtin::BIsprintf ? 1 : 3;
@ -898,7 +771,15 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
SourceSize.getValue().ule(DestinationSize.getValue()))
return;
StringRef FunctionName = GetFunctionName();
StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
// Skim off the details of whichever builtin was called to produce a better
// diagnostic, as it's unlikely that the user wrote the __builtin explicitly.
if (IsChkVariant) {
FunctionName = FunctionName.drop_front(std::strlen("__builtin___"));
FunctionName = FunctionName.drop_back(std::strlen("_chk"));
} else if (FunctionName.startswith("__builtin_")) {
FunctionName = FunctionName.drop_front(std::strlen("__builtin_"));
}
SmallString<16> DestinationStr;
SmallString<16> SourceStr;

63
clang/test/Sema/warn-fortify-scanf.c
View File

@ -1,63 +0,0 @@
// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
typedef struct _FILE FILE;
extern int scanf(const char *format, ...);
extern int fscanf(FILE *f, const char *format, ...);
extern int sscanf(const char *input, const char *format, ...);
void call_scanf() {
char buf10[10];
char buf20[20];
char buf30[30];
scanf("%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding specifier may require size 11}}
scanf("%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding specifier may require size 12}}
scanf("%4s %5s %9s", buf20, buf30, buf10);
scanf("%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding specifier may require size 21}}
scanf("%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding specifier may require size 22}}
scanf("%19s %5s %9s", buf20, buf30, buf10);
scanf("%19s %29s %9s", buf20, buf30, buf10);
scanf("%4[a] %5[a] %10[a]", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding specifier may require size 11}}
scanf("%4[a] %5[a] %11[a]", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding specifier may require size 12}}
scanf("%4[a] %5[a] %9[a]", buf20, buf30, buf10);
scanf("%20[a] %5[a] %9[a]", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding specifier may require size 21}}
scanf("%21[a] %5[a] %9[a]", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding specifier may require size 22}}
scanf("%19[a] %5[a] %9[a]", buf20, buf30, buf10);
scanf("%19[a] %29[a] %9[a]", buf20, buf30, buf10);
scanf("%4c %5c %10c", buf20, buf30, buf10);
scanf("%4c %5c %11c", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 4 has size 10, but the corresponding specifier may require size 11}}
scanf("%4c %5c %9c", buf20, buf30, buf10);
scanf("%20c %5c %9c", buf20, buf30, buf10);
scanf("%21c %5c %9c", buf20, buf30, buf10); // expected-warning {{'scanf' may overflow; destination buffer in argument 2 has size 20, but the corresponding specifier may require size 21}}
// Don't warn for other specifiers.
int x;
scanf("%12d", &x);
}
void call_sscanf() {
char buf10[10];
char buf20[20];
char buf30[30];
sscanf("a b c", "%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding specifier may require size 11}}
sscanf("a b c", "%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding specifier may require size 12}}
sscanf("a b c", "%4s %5s %9s", buf20, buf30, buf10);
sscanf("a b c", "%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding specifier may require size 21}}
sscanf("a b c", "%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'sscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding specifier may require size 22}}
sscanf("a b c", "%19s %5s %9s", buf20, buf30, buf10);
sscanf("a b c", "%19s %29s %9s", buf20, buf30, buf10);
}
void call_fscanf() {
char buf10[10];
char buf20[20];
char buf30[30];
fscanf(0, "%4s %5s %10s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding specifier may require size 11}}
fscanf(0, "%4s %5s %11s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 5 has size 10, but the corresponding specifier may require size 12}}
fscanf(0, "%4s %5s %9s", buf20, buf30, buf10);
fscanf(0, "%20s %5s %9s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding specifier may require size 21}}
fscanf(0, "%21s %5s %9s", buf20, buf30, buf10); // expected-warning {{'fscanf' may overflow; destination buffer in argument 3 has size 20, but the corresponding specifier may require size 22}}
fscanf(0, "%19s %5s %9s", buf20, buf30, buf10);
fscanf(0, "%19s %29s %9s", buf20, buf30, buf10);
}