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

Add a clang-tidy check that flags string-to-number conversion functions that have insufficient error checking, suggesting a better alternative. 

This check corresponds to: https://www.securecoding.cert.org/confluence/display/c/ERR34-C.+Detect+errors+when+converting+a+string+to+a+number

llvm-svn: 268100
This commit is contained in:
Aaron Ballman 2016-04-29 20:56:48 +00:00
parent 9190b4add8
commit d744e63d90
8 changed files with 452 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
View File

@ -20,6 +20,7 @@
#include "FloatLoopCounter.h" #include "FloatLoopCounter.h"
#include "SetLongJmpCheck.h" #include "SetLongJmpCheck.h"
#include "StaticObjectExceptionCheck.h" #include "StaticObjectExceptionCheck.h"
#include "StrToNumCheck.h"
#include "ThrownExceptionTypeCheck.h" #include "ThrownExceptionTypeCheck.h"
#include "VariadicFunctionDefCheck.h" #include "VariadicFunctionDefCheck.h"
@ -64,6 +65,9 @@ public:
// FIO // FIO
CheckFactories.registerCheck<NonCopyableObjectsCheck>( CheckFactories.registerCheck<NonCopyableObjectsCheck>(
"cert-fio38-c"); "cert-fio38-c");
// ERR
CheckFactories.registerCheck<StrToNumCheck>(
"cert-err34-c");
} }
}; };

2
clang-tools-extra/clang-tidy/cert/CMakeLists.txt
View File

@ -6,10 +6,12 @@ add_clang_library(clangTidyCERTModule
FloatLoopCounter.cpp FloatLoopCounter.cpp
SetLongJmpCheck.cpp SetLongJmpCheck.cpp
StaticObjectExceptionCheck.cpp StaticObjectExceptionCheck.cpp
StrToNumCheck.cpp
ThrownExceptionTypeCheck.cpp ThrownExceptionTypeCheck.cpp
VariadicFunctionDefCheck.cpp VariadicFunctionDefCheck.cpp
LINK_LIBS LINK_LIBS
clangAnalysis
clangAST clangAST
clangASTMatchers clangASTMatchers
clangBasic clangBasic

235
clang-tools-extra/clang-tidy/cert/StrToNumCheck.cpp Normal file
View File

@ -0,0 +1,235 @@
//===--- Err34CCheck.cpp - clang-tidy--------------------------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#include "StrToNumCheck.h"
#include "clang/Analysis/Analyses/FormatString.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/StringSwitch.h"
#include <cassert>
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
namespace cert {
void StrToNumCheck::registerMatchers(MatchFinder *Finder) {
// Match any function call to the C standard library string conversion
// functions that do no error checking.
Finder->addMatcher(
callExpr(
callee(functionDecl(anyOf(
functionDecl(hasAnyName("::atoi", "::atof", "::atol", "::atoll"))
.bind("converter"),
functionDecl(hasAnyName("::scanf", "::sscanf", "::fscanf",
"::vfscanf", "::vscanf", "::vsscanf"))
.bind("formatted")))))
.bind("expr"),
this);
}
namespace {
enum class ConversionKind {
None,
ToInt,
ToUInt,
ToLongInt,
ToLongUInt,
ToIntMax,
ToUIntMax,
ToFloat,
ToDouble,
ToLongDouble
};
ConversionKind ClassifyConversionFunc(const FunctionDecl *FD) {
return llvm::StringSwitch<ConversionKind>(FD->getName())
.Cases("atoi", "atol", ConversionKind::ToInt)
.Case("atoll", ConversionKind::ToLongInt)
.Case("atof", ConversionKind::ToDouble)
.Default(ConversionKind::None);
}
ConversionKind ClassifyFormatString(StringRef Fmt, const LangOptions &LO,
const TargetInfo &TI) {
// Scan the format string for the first problematic format specifier, then
// report that as the conversion type. This will miss additional conversion
// specifiers, but that is acceptable behavior.
class Handler : public analyze_format_string::FormatStringHandler {
ConversionKind CK;
bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
const char *startSpecifier,
unsigned specifierLen) override {
// If we just consume the argument without assignment, we don't care
// about it having conversion errors.
if (!FS.consumesDataArgument())
return true;
// Get the conversion specifier and use it to determine the conversion
// kind.
analyze_scanf::ScanfConversionSpecifier SCS = FS.getConversionSpecifier();
if (SCS.isIntArg()) {
switch (FS.getLengthModifier().getKind()) {
case analyze_scanf::LengthModifier::AsLongLong:
CK = ConversionKind::ToLongInt;
break;
case analyze_scanf::LengthModifier::AsIntMax:
CK = ConversionKind::ToIntMax;
break;
default:
CK = ConversionKind::ToInt;
break;
}
} else if (SCS.isUIntArg()) {
switch (FS.getLengthModifier().getKind()) {
case analyze_scanf::LengthModifier::AsLongLong:
CK = ConversionKind::ToLongUInt;
break;
case analyze_scanf::LengthModifier::AsIntMax:
CK = ConversionKind::ToUIntMax;
break;
default:
CK = ConversionKind::ToUInt;
break;
}
} else if (SCS.isDoubleArg()) {
switch (FS.getLengthModifier().getKind()) {
case analyze_scanf::LengthModifier::AsLongDouble:
CK = ConversionKind::ToLongDouble;
break;
case analyze_scanf::LengthModifier::AsLong:
CK = ConversionKind::ToDouble;
break;
default:
CK = ConversionKind::ToFloat;
break;
}
}
// Continue if we have yet to find a conversion kind that we care about.
return CK == ConversionKind::None;
}
public:
Handler() : CK(ConversionKind::None) {}
ConversionKind get() const { return CK; }
};
Handler H;
analyze_format_string::ParseScanfString(H, Fmt.begin(), Fmt.end(), LO, TI);
return H.get();
}
StringRef ClassifyConversionType(ConversionKind K) {
switch (K) {
case ConversionKind::None:
assert(false && "Unexpected conversion kind");
case ConversionKind::ToInt:
case ConversionKind::ToLongInt:
case ConversionKind::ToIntMax:
return "an integer value";
case ConversionKind::ToUInt:
case ConversionKind::ToLongUInt:
case ConversionKind::ToUIntMax:
return "an unsigned integer value";
case ConversionKind::ToFloat:
case ConversionKind::ToDouble:
case ConversionKind::ToLongDouble:
return "a floating-point value";
}
llvm_unreachable("Unknown conversion kind");
}
StringRef ClassifyReplacement(ConversionKind K) {
switch (K) {
case ConversionKind::None:
assert(false && "Unexpected conversion kind");
case ConversionKind::ToInt:
return "strtol";
case ConversionKind::ToUInt:
return "strtoul";
case ConversionKind::ToIntMax:
return "strtoimax";
case ConversionKind::ToLongInt:
return "strtoll";
case ConversionKind::ToLongUInt:
return "strtoull";
case ConversionKind::ToUIntMax:
return "strtoumax";
case ConversionKind::ToFloat:
return "strtof";
case ConversionKind::ToDouble:
return "strtod";
case ConversionKind::ToLongDouble:
return "strtold";
}
llvm_unreachable("Unknown conversion kind");
}
} // unnamed namespace
void StrToNumCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Call = Result.Nodes.getNodeAs<CallExpr>("expr");
const FunctionDecl *FuncDecl = nullptr;
ConversionKind Conversion;
if (const auto *ConverterFunc =
Result.Nodes.getNodeAs<FunctionDecl>("converter")) {
// Converter functions are always incorrect to use.
FuncDecl = ConverterFunc;
Conversion = ClassifyConversionFunc(ConverterFunc);
} else if (const auto *FFD =
Result.Nodes.getNodeAs<FunctionDecl>("formatted")) {
StringRef FmtStr;
// The format string comes from the call expression and depends on which
// flavor of scanf is called.
// Index 0: scanf, vscanf, Index 1: fscanf, sscanf, vfscanf, vsscanf.
unsigned Idx =
(FFD->getName() == "scanf" || FFD->getName() == "vscanf") ? 0 : 1;
// Given the index, see if the call expression argument at that index is
// a string literal.
if (Call->getNumArgs() < Idx)
return;
if (const Expr *Arg = Call->getArg(Idx)->IgnoreParenImpCasts()) {
if (const auto *SL = dyn_cast<StringLiteral>(Arg)) {
FmtStr = SL->getString();
}
}
// If we could not get the format string, bail out.
if (FmtStr.empty())
return;
// Formatted input functions need further checking of the format string to
// determine whether a problematic conversion may be happening.
Conversion = ClassifyFormatString(FmtStr, Result.Context->getLangOpts(),
Result.Context->getTargetInfo());
if (Conversion != ConversionKind::None)
FuncDecl = FFD;
}
if (!FuncDecl)
return;
diag(Call->getExprLoc(),
"%0 used to convert a string to %1, but function will not report "
"conversion errors; consider using '%2' instead")
<< FuncDecl << ClassifyConversionType(Conversion)
<< ClassifyReplacement(Conversion);
}
} // namespace cert
} // namespace tidy
} // namespace clang

36
clang-tools-extra/clang-tidy/cert/StrToNumCheck.h Normal file
View File

@ -0,0 +1,36 @@
//===--- StrToNumCheck.h - clang-tidy----------------------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_STRTONUMCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_STRTONUMCHECK_H
#include "../ClangTidy.h"
namespace clang {
namespace tidy {
namespace cert {
/// Guards against use of string conversion functions that do not have
/// reasonable error handling for conversion errors.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/cert-err34-c.html
class StrToNumCheck : public ClangTidyCheck {
public:
StrToNumCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};
} // namespace cert
} // namespace tidy
} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_STRTONUMCHECK_H

28
clang-tools-extra/docs/clang-tidy/checks/cert-err34-c.rst Normal file
View File

@ -0,0 +1,28 @@
.. title:: clang-tidy - cert-err34-c
cert-err34-c
============
This check flags calls to string-to-number conversion functions that do not
verify the validity of the conversion, such as ``atoi()`` or ``scanf()``. It
does not flag calls to ``strtol()``, or other, related conversion functions that
do perform better error checking.
.. code:: c
#include <stdlib.h>
void func(const char *buff) {
int si;
if (buff) {
si = atoi(buff); /* 'atoi' used to convert a string to an integer, but function will
not report conversion errors; consider using 'strtol' instead. */
} else {
/* Handle error */
}
}
This check corresponds to the CERT C Coding Standard rule
`ERR34-C. Detect errors when converting a string to a number
<https://www.securecoding.cert.org/confluence/display/c/ERR34-C.+Detect+errors+when+converting+a+string+to+a+number>`_.

1
clang-tools-extra/docs/clang-tidy/checks/list.rst
View File

@ -11,6 +11,7 @@ Clang-Tidy Checks
cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp> cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp>
cert-dcl59-cpp (redirects to google-build-namespaces) <cert-dcl59-cpp> cert-dcl59-cpp (redirects to google-build-namespaces) <cert-dcl59-cpp>
cert-env33-c cert-env33-c
cert-err34-c
cert-err52-cpp cert-err52-cpp
cert-err58-cpp cert-err58-cpp
cert-err60-cpp cert-err60-cpp

103
clang-tools-extra/test/clang-tidy/cert-err34-c.c Normal file
View File

@ -0,0 +1,103 @@
// RUN: %check_clang_tidy %s cert-err34-c %t -- -- -std=c11
typedef __SIZE_TYPE__ size_t;
typedef signed ptrdiff_t;
typedef long long intmax_t;
typedef unsigned long long uintmax_t;
typedef void * FILE;
extern FILE *stdin;
extern int fscanf(FILE * restrict stream, const char * restrict format, ...);
extern int scanf(const char * restrict format, ...);
extern int sscanf(const char * restrict s, const char * restrict format, ...);
extern double atof(const char *nptr);
extern int atoi(const char *nptr);
extern long int atol(const char *nptr);
extern long long int atoll(const char *nptr);
void f1(const char *in) {
int i;
long long ll;
unsigned int ui;
unsigned long long ull;
intmax_t im;
uintmax_t uim;
float f;
double d;
long double ld;
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'sscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
sscanf(in, "%d", &i);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'fscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtoll' instead [cert-err34-c]
fscanf(stdin, "%lld", &ll);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'sscanf' used to convert a string to an unsigned integer value, but function will not report conversion errors; consider using 'strtoul' instead [cert-err34-c]
sscanf(in, "%u", &ui);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'fscanf' used to convert a string to an unsigned integer value, but function will not report conversion errors; consider using 'strtoull' instead [cert-err34-c]
fscanf(stdin, "%llu", &ull);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtoimax' instead [cert-err34-c]
scanf("%jd", &im);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'fscanf' used to convert a string to an unsigned integer value, but function will not report conversion errors; consider using 'strtoumax' instead [cert-err34-c]
fscanf(stdin, "%ju", &uim);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'sscanf' used to convert a string to a floating-point value, but function will not report conversion errors; consider using 'strtof' instead [cert-err34-c]
sscanf(in, "%f", &f); // to float
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'fscanf' used to convert a string to a floating-point value, but function will not report conversion errors; consider using 'strtod' instead [cert-err34-c]
fscanf(stdin, "%lg", &d);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'sscanf' used to convert a string to a floating-point value, but function will not report conversion errors; consider using 'strtold' instead [cert-err34-c]
sscanf(in, "%Le", &ld);
// These are conversions with other modifiers
short s;
char c;
size_t st;
ptrdiff_t pt;
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%hhd", &c);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%hd", &s);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%zu", &st);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%td", &pt);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%o", ui);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%X", ui);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%x", ui);
}
void f2(const char *in) {
// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: 'atoi' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
int i = atoi(in); // to int
// CHECK-MESSAGES: :[[@LINE+1]]:12: warning: 'atol' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
long l = atol(in); // to long
// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: 'atoll' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtoll' instead [cert-err34-c]
long long ll = atoll(in); // to long long
// CHECK-MESSAGES: :[[@LINE+1]]:14: warning: 'atof' used to convert a string to a floating-point value, but function will not report conversion errors; consider using 'strtod' instead [cert-err34-c]
double d = atof(in); // to double
}
void f3(void) {
int i;
unsigned int u;
float f;
char str[32];
// Test that we don't report multiple infractions for a single call.
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%d%u%f", &i, &u, &f);
// Test that we still catch infractions that are not the first specifier.
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'scanf' used to convert
scanf("%s%d", str, &i);
}
void do_not_diagnose(void) {
char str[32];
scanf("%s", str); // Not a numerical conversion
scanf("%*d"); // Assignment suppressed
}

43
clang-tools-extra/test/clang-tidy/cert-err34-c.cpp Normal file
View File

@ -0,0 +1,43 @@
// RUN: %check_clang_tidy %s cert-err34-c %t -- -- -std=c++11
typedef void * FILE;
extern FILE *stdin;
extern int fscanf(FILE * stream, const char * format, ...);
extern int sscanf(const char * s, const char * format, ...);
extern double atof(const char *nptr);
extern int atoi(const char *nptr);
extern long int atol(const char *nptr);
extern long long int atoll(const char *nptr);
namespace std {
using ::FILE; using ::stdin;
using ::fscanf; using ::sscanf;
using ::atof; using ::atoi; using ::atol; using ::atoll;
}
void f1(const char *in) {
int i;
long long ll;
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'sscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
std::sscanf(in, "%d", &i);
// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: 'fscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtoll' instead [cert-err34-c]
std::fscanf(std::stdin, "%lld", &ll);
}
void f2(const char *in) {
// CHECK-MESSAGES: :[[@LINE+1]]:11: warning: 'atoi' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
int i = std::atoi(in); // to int
// CHECK-MESSAGES: :[[@LINE+1]]:12: warning: 'atol' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead [cert-err34-c]
long l = std::atol(in); // to long
using namespace std;
// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: 'atoll' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtoll' instead [cert-err34-c]
long long ll = atoll(in); // to long long
// CHECK-MESSAGES: :[[@LINE+1]]:14: warning: 'atof' used to convert a string to a floating-point value, but function will not report conversion errors; consider using 'strtod' instead [cert-err34-c]
double d = atof(in); // to double
}