Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -21,6 +21,7 @@ #include "FloatLoopCounter.h" #include "LimitedRandomnessCheck.h" #include "PostfixOperatorCheck.h" +#include "ProperlySeededRandomGeneratorCheck.h" #include "SetLongJmpCheck.h" #include "StaticObjectExceptionCheck.h" #include "StrToNumCheck.h" @@ -36,13 +37,11 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { // C++ checkers // DCL - CheckFactories.registerCheck( - "cert-dcl21-cpp"); + CheckFactories.registerCheck("cert-dcl21-cpp"); CheckFactories.registerCheck("cert-dcl50-cpp"); CheckFactories.registerCheck( "cert-dcl54-cpp"); - CheckFactories.registerCheck( - "cert-dcl58-cpp"); + CheckFactories.registerCheck("cert-dcl58-cpp"); CheckFactories.registerCheck( "cert-dcl59-cpp"); // OOP @@ -58,6 +57,8 @@ "cert-err61-cpp"); // MSC CheckFactories.registerCheck("cert-msc50-cpp"); + CheckFactories.registerCheck( + "cert-msc51-cpp"); // C checkers // DCL @@ -72,6 +73,8 @@ CheckFactories.registerCheck("cert-err34-c"); // MSC CheckFactories.registerCheck("cert-msc30-c"); + CheckFactories.registerCheck( + "cert-msc32-c"); } }; Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -7,6 +7,7 @@ FloatLoopCounter.cpp LimitedRandomnessCheck.cpp PostfixOperatorCheck.cpp + ProperlySeededRandomGeneratorCheck.cpp SetLongJmpCheck.cpp StaticObjectExceptionCheck.cpp StrToNumCheck.cpp Index: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h =================================================================== --- /dev/null +++ clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h @@ -0,0 +1,47 @@ +//===--- ProperlySeededRandomGeneratorCheck.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_PROPERLY_SEEDED_RANDOM_GENERATOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLY_SEEDED_RANDOM_GENERATOR_H + +#include "../ClangTidy.h" +#include + +namespace clang { +namespace tidy { +namespace cert { + +/// Random number generator must be seeded properly. +/// +/// A random number generator initialized with default value or a +/// constant expression is a security vulnerability. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-properly-seeded-random-generator.html +class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck { +public: + ProperlySeededRandomGeneratorCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + template + void checkSeed(const ast_matchers::MatchFinder::MatchResult &Result, + const T *Func); + + std::string RawDisallowedSeedTypes; + SmallVector DisallowedSeedTypes; +}; + +} // namespace cert +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLY_SEEDED_RANDOM_GENERATOR_H Index: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp @@ -0,0 +1,125 @@ +//===--- ProperlySeededRandomGeneratorCheck.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 "ProperlySeededRandomGeneratorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cert { + +ProperlySeededRandomGeneratorCheck::ProperlySeededRandomGeneratorCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + RawDisallowedSeedTypes(Options.get("DisallowedSeedTypes", "")) { + StringRef(RawDisallowedSeedTypes).split(DisallowedSeedTypes, ','); +} + +void ProperlySeededRandomGeneratorCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "DisallowedSeedTypes", RawDisallowedSeedTypes); +} + +void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) { + auto RandomGeneratorEngineDecl = recordDecl( + hasAnyName("linear_congruential_engine", "mersenne_twister_engine", + "subtract_with_carry_engine", "discard_block_engine", + "independent_bits_engine", "shuffle_order_engine")); + auto RandomGeneratorEngineType = + qualType(hasCanonicalType(hasDeclaration(RandomGeneratorEngineDecl))); + + // std::mt19937 engine; + // engine.seed(); + // ^ + // engine.seed(1); + // ^ + // const int x = 1; + // engine.seed(x); + // ^ + Finder->addMatcher( + cxxMemberCallExpr( + has(memberExpr(has(declRefExpr(hasType(RandomGeneratorEngineType))), + member(hasName("seed")), + unless(hasDescendant(cxxThisExpr()))))) + .bind("seed"), + this); + + // std::mt19937 engine; + // ^ + // std::mt19937 engine(1); + // ^ + // const int x = 1; + // std::mt19937 engine(x); + // ^ + Finder->addMatcher( + cxxConstructExpr(hasType(RandomGeneratorEngineType)).bind("ctor"), this); + + // srand(); + // ^ + // const int x = 1; + // srand(x); + // ^ + Finder->addMatcher( + callExpr(has(implicitCastExpr(has( + declRefExpr(hasDeclaration(namedDecl(hasName("srand")))))))) + .bind("srand"), + this); +} + +void ProperlySeededRandomGeneratorCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Ctor = Result.Nodes.getNodeAs("ctor"); + if (Ctor) + checkSeed(Result, Ctor); + + const auto *Func = Result.Nodes.getNodeAs("seed"); + if (Func) + checkSeed(Result, Func); + + const auto *Srand = Result.Nodes.getNodeAs("srand"); + if (Srand) + checkSeed(Result, Srand); +} + +template +void ProperlySeededRandomGeneratorCheck::checkSeed( + const MatchFinder::MatchResult &Result, const T *Func) { + if (Func->getNumArgs() == 0 || Func->getArg(0)->isDefaultArgument()) { + diag(Func->getExprLoc(), + "random number generator seeded with a default argument will generate " + "a predictable sequence of values"); + return; + } + + llvm::APSInt Value; + if (Func->getArg(0)->EvaluateAsInt(Value, *Result.Context)) { + diag(Func->getExprLoc(), + "random number generator seeded with a constant value will generate a " + "predictable sequence of values"); + return; + } + + const std::string SeedType( + Func->getArg(0)->IgnoreCasts()->getType().getAsString()); + if (std::find(DisallowedSeedTypes.begin(), DisallowedSeedTypes.end(), + SeedType) != DisallowedSeedTypes.end()) { + diag(Func->getExprLoc(), + "random number generator seeded with a disallowed source of seed " + "value will generate a predictable sequence of values"); + return; + } +} + +} // namespace cert +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New `cert-msc51-cpp + `_ check + + Detects inappropriate seeding of C++ random generators and C srand function. + - The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings `_ Index: docs/clang-tidy/checks/cert-msc32-c.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cert-msc32-c.rst @@ -0,0 +1,9 @@ +.. title:: clang-tidy - cert-msc32-c +.. meta:: + :http-equiv=refresh: 5;URL=cert-msc51-cpp.html + +cert-msc32-c +============ + +The cert-msc32-c check is an alias, please see +`cert-msc51-cpp `_ for more information. Index: docs/clang-tidy/checks/cert-msc51-cpp.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cert-msc51-cpp.rst @@ -0,0 +1,35 @@ +.. title:: clang-tidy - cert-msc51-cpp + +cert-msc51-cpp +============== + +This check flags all pseudo-random number engines, engine adaptor +instantiations and srand when initialized or seeded with default argument, +constant expression or any user-configurable type. Pseudo-random number +engines seeded with a predictable value may cause vulnerabilities e.g. in +security protocols. +This is a CERT security rule, see MSC51-CPP. + +Examples: + +.. code-block:: c++ + + void foo() { + std::mt19937 engine1; // Bad, always generate the same sequence + std::mt19937 engine2(1); // Bad + engine1.seed(); // Bad + engine2.seed(1); // Bad + + std::time_t t; + engine1.seed(std::time(&t)); // Bad, system time might be controlled by user + + int x = atoi(argv[1]); + std::mt19937 engine3(x); // Will not warn + } + +Options +------- + +.. option:: DisallowedSeedTypes + + A comma-separated list of the type names which are disallowed. Index: test/clang-tidy/cert-msc32-c.c =================================================================== --- /dev/null +++ test/clang-tidy/cert-msc32-c.c @@ -0,0 +1,27 @@ +// RUN: %check_clang_tidy %s cert-msc32-c %t -- -config="{CheckOptions: [{key: cert-msc32-c.DisallowedSeedTypes, value: 'some_type,time_t'}]}" -- -std=c99 + +void srand(int seed); +typedef int time_t; +time_t time(time_t *t); + +void f() { + srand(1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc32-c] + + const int a = 1; + srand(a); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc32-c] + + time_t t; + srand(time(&t)); // Disallowed seed type + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc32-c] +} + +void g() { + typedef int user_t; + user_t a = 1; + srand(a); + + int b = 1; + srand(b); // Can not evaluate as int +} Index: test/clang-tidy/cert-msc51-cpp.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cert-msc51-cpp.cpp @@ -0,0 +1,201 @@ +// RUN: %check_clang_tidy %s cert-msc51-cpp %t -- -config="{CheckOptions: [{key: cert-msc51-cpp.DisallowedSeedTypes, value: 'some_type,time_t'}]}" -- -std=c++11 + +namespace std { + +template +struct linear_congruential_engine { + linear_congruential_engine(int _ = 0); + void seed(int _ = 0); +}; +using default_random_engine = linear_congruential_engine; + +using size_t = int; +template +struct mersenne_twister_engine { + mersenne_twister_engine(int _ = 0); + void seed(int _ = 0); +}; +using mt19937 = mersenne_twister_engine; + +template +struct subtract_with_carry_engine { + subtract_with_carry_engine(int _ = 0); + void seed(int _ = 0); +}; +using ranlux24_base = subtract_with_carry_engine; + +template +struct discard_block_engine { + discard_block_engine(); + discard_block_engine(int _); + void seed(); + void seed(int _); +}; +using ranlux24 = discard_block_engine; + +template +struct independent_bits_engine { + independent_bits_engine(); + independent_bits_engine(int _); + void seed(); + void seed(int _); +}; +using independent_bits = independent_bits_engine; + +template +struct shuffle_order_engine { + shuffle_order_engine(); + shuffle_order_engine(int _); + void seed(); + void seed(int _); +}; +using shuffle_order = shuffle_order_engine; + +struct random_device { + random_device(); + int operator()(); +}; +} // namespace std + +using time_t = unsigned int; +time_t time(time_t *t); + +void f() { + const int seed = 2; + time_t t; + + // One instantiation for every engine + std::default_random_engine engine1; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::default_random_engine engine2(1); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::default_random_engine engine3(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::default_random_engine engine4(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine1.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine1.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine1.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine1.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + + std::mt19937 engine5; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::mt19937 engine6(1); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::mt19937 engine7(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::mt19937 engine8(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine5.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine5.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine5.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine5.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + + std::ranlux24_base engine9; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24_base engine10(1); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24_base engine11(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24_base engine12(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine9.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine9.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine9.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine9.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + + std::ranlux24 engine13; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24 engine14(1); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24 engine15(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::ranlux24 engine16(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine13.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine13.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine13.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine13.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + + std::independent_bits engine17; + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::independent_bits engine18(1); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::independent_bits engine19(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::independent_bits engine20(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine17.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine17.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine17.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine17.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + + std::shuffle_order engine21; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + std::shuffle_order engine22(1); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::shuffle_order engine23(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + std::shuffle_order engine24(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] + engine21.seed(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a default argument will generate a predictable sequence of values [cert-msc51-cpp] + engine21.seed(1); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine21.seed(seed); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a constant value will generate a predictable sequence of values [cert-msc51-cpp] + engine21.seed(time(&t)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [cert-msc51-cpp] +} + +struct A { + A(int _ = 0); + void seed(int _ = 0); +}; + +void g() { + int n = 1; + std::default_random_engine engine1(n); + std::mt19937 engine2(n); + std::ranlux24_base engine3(n); + std::ranlux24 engine4(n); + std::independent_bits engine5(n); + std::shuffle_order engine6(n); + + std::random_device dev; + std::default_random_engine engine7(dev()); + std::mt19937 engine8(dev()); + std::ranlux24_base engine9(dev()); + std::ranlux24 engine10(dev()); + std::independent_bits engine11(dev()); + std::shuffle_order engine12(dev()); + + A a1; + A a2(1); + a1.seed(); + a1.seed(1); + a1.seed(n); +}