Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -8521,6 +8521,10 @@ ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, CallExpr *TheCall); + void CheckSprintfMemAlloc(FunctionDecl *FDecl, CallExpr *TheCall, + unsigned BufIdx, unsigned FmtIdx, + unsigned DataIdx); + bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall, unsigned MaxWidth); bool CheckNeonBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); @@ -8562,6 +8566,7 @@ enum FormatStringType { FST_Scanf, FST_Printf, + FST_Sprintf, FST_NSString, FST_Strftime, FST_Strfmon, Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -480,6 +480,9 @@ case Builtin::BI__builtin___stpncpy_chk: SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3); break; + case Builtin::BI__builtin___sprintf_chk: + CheckSprintfMemAlloc(FDecl, TheCall, 0, 3, 4); + break; case Builtin::BI__builtin___memccpy_chk: SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4); break; @@ -1317,6 +1320,40 @@ return false; } +void Sema::CheckSprintfMemAlloc(FunctionDecl *FDecl, CallExpr *TheCall, + unsigned BufIdx, unsigned FmtIdx, + unsigned DataIdx) { + unsigned NumArgs = TheCall->getNumArgs(); + if (NumArgs <= BufIdx || + NumArgs <= FmtIdx || + NumArgs <= DataIdx) + return; + + const Expr *FmtArg = TheCall->getArg(FmtIdx); + if (!isa(FmtArg->IgnoreImplicit())) + return; + + const FunctionProtoType *Proto = + FDecl->getType()->castAs(); + VariadicCallType CallType = getVariadicCallType(FDecl, Proto, + TheCall->getCallee()); + llvm::ArrayRef Args = + llvm::makeArrayRef(TheCall->getArgs(), NumArgs); + llvm::SmallBitVector CheckedVarArgs; + if (FDecl) { + for (const auto *I : FDecl->specific_attrs()) { + // Only create vector if there are format attributes. + CheckedVarArgs.resize(Args.size()); + CheckFormatArguments(I, Args, /*IsMemberFunction=*/false, CallType, + TheCall->getLocStart(), FDecl->getSourceRange(), + CheckedVarArgs); + } + } + const StringLiteral *SL = cast(FmtArg->IgnoreImplicit()); + CheckFormatString(SL, FmtArg, Args, true, FmtIdx, DataIdx, FST_Sprintf, + /*inFunctionCall=*/true, CallType, CheckedVarArgs); +} + bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac, ArrayRef Args) { VariadicCallType CallType = @@ -3355,7 +3392,7 @@ bool checkForCStrMembers(const analyze_printf::ArgType &AT, const Expr *E); -}; +}; } bool CheckPrintfHandler::HandleInvalidPrintfConversionSpecifier( @@ -4050,6 +4087,177 @@ return true; } +static const ConstantArrayType* GetArrayType(Sema &S, const Expr *Array) { + if (isa(Array)) { + QualType ArrayTy = cast(Array)->getDecl()->getType(); + if (ArrayTy->isConstantArrayType()) + return S.Context.getAsConstantArrayType(Array->getType()); + } + return nullptr; +} + +// CHECK: Sprintf format string checking, uses printf checking with added data +// size analysing to warn of memory overflows. +namespace { + class CheckSprintfHandler : public CheckPrintfHandler { + public: + CheckSprintfHandler(Sema &S, const StringLiteral *FExpr, + const Expr *OrigFormatExpr, unsigned FirstDataArg, + unsigned numDataArgs, bool isObjC, + const char *Begin, bool hasVAListArg, + ArrayRef Args, + unsigned FormatIdx, bool inFunctionCall, + Sema::VariadicCallType CallType, + llvm::SmallBitVector &CheckedVarArgs) + : CheckPrintfHandler(S, FExpr, OrigFormatExpr, FirstDataArg, numDataArgs, + isObjC, Begin, hasVAListArg, Args, FormatIdx, + inFunctionCall, CallType, CheckedVarArgs), + Args(Args), S(S) { + StorageBufSize = llvm::APInt(64, 0); + StorageRequired = llvm::APInt(64, 0); + } + + bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, + unsigned specifierLen) override; + + bool Init() { + if (const ConstantArrayType *StorageBufTy = + GetArrayType(S, Args[0]->IgnoreImplicit())) { + StorageBufSize = StorageBufTy->getSize(); + return true; + } + return false; + } + + private: + ArrayRef Args; + Sema &S; + llvm::APInt StorageBufSize; + llvm::APInt StorageRequired; + }; +} + +// Analyses the data arguments to sprintf to warn the user of memory overflows. +// Mainly handles integer and string literals with their specifiers and flags. +bool +CheckSprintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier + &FS, const char *startSpecifier, + unsigned specifierLen) { + // Call the parent call first to do perform the validation on the specifier. + if (!CheckPrintfHandler::HandlePrintfSpecifier(FS, startSpecifier, + specifierLen)) + return false; + + using namespace analyze_format_string; + using namespace analyze_printf; + + const PrintfConversionSpecifier &CS = FS.getConversionSpecifier(); + ConversionSpecifier::Kind CSKind = CS.getKind(); + OptionalAmount Precision = FS.getPrecision(); + OptionalAmount Width = FS.getFieldWidth(); + + // Offset the argument index to the data items to be printed. + unsigned Idx = 4 + FS.getArgIndex(); + const Expr* Data = Args[Idx]->IgnoreImplicit(); + + // An extra will be required for +/- + if (FS.hasPlusPrefix() || FS.hasSpacePrefix()) + ++StorageRequired; + + if (isa(Data)) { + auto SL = cast(Data); + StorageRequired += llvm::APInt(64, SL->getByteLength()); + } + else if (isa(Data)) { + // Default to decimal representation. + unsigned Radix = 10; + + // Unsigned hexadecimal integers and pointer addresses. + if (CSKind == ConversionSpecifier::xArg || + CSKind == ConversionSpecifier::XArg || + CSKind == ConversionSpecifier::pArg) + Radix = 16; + + // Unsigned octal. + else if (CSKind == ConversionSpecifier::oArg || + CSKind == ConversionSpecifier::OArg) + Radix = 8; + + // Create a real string to use its size. + std::string DataStr = + cast(Data)->getValue().toString(Radix, true); + StorageRequired += llvm::APInt(64, DataStr.length()); + } + else if (isa(Data)) { + llvm::APFloat Value = cast(Data)->getValue(); + + // APFloat has a member function for printing the value to a hex + // representation. + if (CSKind == ConversionSpecifier::aArg || + CSKind == ConversionSpecifier::AArg) { + char TempBuf[32]; + unsigned HexDigits = 0; + + if (Precision.getHowSpecified() == OptionalAmount::Constant) + HexDigits = Precision.getConstantAmount(); + + // Get the number of digits used to print the value. + unsigned NumDigits = + Value.convertToHexString(TempBuf, HexDigits, false, + llvm::APFloat::rmNearestTiesToEven); + StorageRequired += llvm::APInt(64, NumDigits); + } + } + else if (auto DataBuf = GetArrayType(S, Data)) { + llvm::APInt BufSize = DataBuf->getSize(); + StorageRequired += BufSize; + } + + if (FS.hasAlternativeForm()) { + // 0x will be prepended for oct and hex values. + if (CSKind == ConversionSpecifier::xArg || + CSKind == ConversionSpecifier::XArg || + CSKind == ConversionSpecifier::oArg || + CSKind == ConversionSpecifier::OArg) + StorageRequired += llvm::APInt(64, 2); + } + + // Pointer address have 'p' prepended + if (CSKind == ConversionSpecifier::pArg) + ++StorageRequired; + + // The calculated sizes could be smaller than sizes specified by the user + // in their format string. + if (CS.isAnyIntArg()) { + // For integers, the precision modifier specifies how many digits to be + // written so this may overwrite the calculated size. + if (Precision.getHowSpecified() == OptionalAmount::Constant) { + if (StorageRequired.ule(Precision.getConstantAmount())) + StorageRequired = llvm::APInt(64, Precision.getConstantAmount()); + } + } + + // Width specifies the minimum number of characters to be printed. + if (Width.getHowSpecified() == OptionalAmount::Constant) { + if (StorageRequired.ule(Width.getConstantAmount())) + StorageRequired = llvm::APInt(64, Width.getConstantAmount()); + } + + // Compare less or equal to account for the null terminator that will + // be appended. + if (StorageBufSize.ule(StorageRequired)) { + //S.Diag(Args[0]->getLocStart(), diag::warn_sprintf_chk_overflow) + S.Diag(Args[0]->getLocStart(), diag::warn_memcpy_chk_overflow) + << getSpecifierRange(startSpecifier, specifierLen) + << Data->getSourceRange() + << Args[Idx]->getType(); + return false; + } + + return true; +} + //===--- CHECK: Scanf format string checking ------------------------------===// namespace { @@ -4295,6 +4503,22 @@ getLangOpts(), Context.getTargetInfo())) H.DoneProcessing(); + } + else if (Type == FST_Sprintf) { + // Sprintf has the same parsing and checking as printf, but we can also + // perform memory overflow checks. + CheckSprintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, + numDataArgs, + (Type == FST_NSString || Type == FST_OSTrace), + Str, HasVAListArg, Args, format_idx, + inFunctionCall, CallType, CheckedVarArgs); + if (H.Init()) { + if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen, + getLangOpts(), + Context.getTargetInfo(), + Type == FST_FreeBSDKPrintf)) + H.DoneProcessing(); + } } // TODO: handle other formats } Index: test/SemaCXX/sprintf-chk.cpp =================================================================== --- /dev/null +++ test/SemaCXX/sprintf-chk.cpp @@ -0,0 +1,70 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -fsyntax-only %s +void test() { + + char bufA[4]; + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%s", "foo"); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d%d", 10, 10); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i%i", 10, 10); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u%u", 10, 10); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x%x", 100, 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X%X", 100, 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o%o", 100, 100); // expected-warning {{will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%d", 1000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%i", 1000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%u", 1000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%x", 10000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%X", 10000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%o", 1000); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%a", 10.0); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%A", 10.0); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%s", "foobar"); // expected-warning {{will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%+d", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%+d", 10); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3d", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3i", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3u", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3x", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3X", 100); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%3o", 100); + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4d", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4i", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4u", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4x", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4X", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%4o", 100); // expected-warning {{will always overflow destination buffer}} + + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#x", 15); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#X", 15); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#o", 7); + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#x", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#X", 100); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufA, 0, __builtin_object_size(bufA, 0), "%#o", 100); // expected-warning {{will always overflow destination buffer}} + + + char bufB[7]; + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%a", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%A", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%a", 10.0); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%A", 10.0); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.1a", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.1A", 0.5); + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.2a", 0.5); // expected-warning {{will always overflow destination buffer}} + __builtin___sprintf_chk(bufB, 0, __builtin_object_size(bufB, 0), "%1.2A", 0.5); // expected-warning {{will always overflow destination buffer}} + + char bufC[5]; + __builtin___sprintf_chk(bufC, 0, __builtin_object_size(bufC, 0), "%s%s", // expected-warning {{will always overflow destination buffer}} + "foo", "bar"); + const char bar[] = "foobar"; + __builtin___sprintf_chk(bufC, 0, __builtin_object_size(bufC, 0), "%s", bar); // expected-warning {{will always overflow destination buffer}} +}