Skip to content
Open
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#include "ParentVirtualCallCheck.h"
#include "PointerArithmeticOnPolymorphicObjectCheck.h"
#include "PosixReturnCheck.h"
#include "RandomGeneratorSeedCheck.h"
#include "RawMemoryCallOnNonTrivialTypeCheck.h"
#include "RedundantBranchConditionCheck.h"
#include "ReservedIdentifierCheck.h"
Expand Down Expand Up @@ -227,6 +228,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<ParentVirtualCallCheck>(
"bugprone-parent-virtual-call");
CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
CheckFactories.registerCheck<RandomGeneratorSeedCheck>(
"bugprone-random-generator-seed");
CheckFactories.registerCheck<RawMemoryCallOnNonTrivialTypeCheck>(
"bugprone-raw-memory-call-on-non-trivial-type");
CheckFactories.registerCheck<ReservedIdentifierCheck>(
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
ParentVirtualCallCheck.cpp
PointerArithmeticOnPolymorphicObjectCheck.cpp
PosixReturnCheck.cpp
RandomGeneratorSeedCheck.cpp
RawMemoryCallOnNonTrivialTypeCheck.cpp
RedundantBranchConditionCheck.cpp
ReservedIdentifierCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,28 @@
//
//===----------------------------------------------------------------------===//

#include "ProperlySeededRandomGeneratorCheck.h"
#include "RandomGeneratorSeedCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/STLExtras.h"

using namespace clang::ast_matchers;

namespace clang::tidy::cert {
namespace clang::tidy::bugprone {

ProperlySeededRandomGeneratorCheck::ProperlySeededRandomGeneratorCheck(
StringRef Name, ClangTidyContext *Context)
RandomGeneratorSeedCheck::RandomGeneratorSeedCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
RawDisallowedSeedTypes(
Options.get("DisallowedSeedTypes", "time_t,std::time_t")) {
RawDisallowedSeedTypes.split(DisallowedSeedTypes, ',');
}

void ProperlySeededRandomGeneratorCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
void RandomGeneratorSeedCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "DisallowedSeedTypes", RawDisallowedSeedTypes);
}

void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) {
void RandomGeneratorSeedCheck::registerMatchers(MatchFinder *Finder) {
auto RandomGeneratorEngineDecl = cxxRecordDecl(hasAnyName(
"::std::linear_congruential_engine", "::std::mersenne_twister_engine",
"::std::subtract_with_carry_engine", "::std::discard_block_engine",
Expand Down Expand Up @@ -75,8 +74,7 @@ void ProperlySeededRandomGeneratorCheck::registerMatchers(MatchFinder *Finder) {
this);
}

void ProperlySeededRandomGeneratorCheck::check(
const MatchFinder::MatchResult &Result) {
void RandomGeneratorSeedCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
if (Ctor)
checkSeed(Result, Ctor);
Expand All @@ -91,8 +89,8 @@ void ProperlySeededRandomGeneratorCheck::check(
}

template <class T>
void ProperlySeededRandomGeneratorCheck::checkSeed(
const MatchFinder::MatchResult &Result, const T *Func) {
void RandomGeneratorSeedCheck::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 "
Expand All @@ -118,4 +116,4 @@ void ProperlySeededRandomGeneratorCheck::checkSeed(
}
}

} // namespace clang::tidy::cert
} // namespace clang::tidy::bugprone
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H

#include "../ClangTidyCheck.h"
#include <string>

namespace clang::tidy::cert {
namespace clang::tidy::bugprone {

/// 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:
/// https://clang.llvm.org/extra/clang-tidy/checks/cert/msc51-cpp.html
class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/random-generator-seed.html
class RandomGeneratorSeedCheck : public ClangTidyCheck {
public:
ProperlySeededRandomGeneratorCheck(StringRef Name, ClangTidyContext *Context);
RandomGeneratorSeedCheck(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;
Expand All @@ -37,6 +37,6 @@ class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
SmallVector<StringRef, 5> DisallowedSeedTypes;
};

} // namespace clang::tidy::cert
} // namespace clang::tidy::bugprone

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RANDOMGENERATORSEEDCHECK_H
6 changes: 3 additions & 3 deletions clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "../bugprone/DefaultOperatorNewOnOveralignedTypeCheck.h"
#include "../bugprone/FloatLoopCounterCheck.h"
#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
#include "../bugprone/RandomGeneratorSeedCheck.h"
#include "../bugprone/RawMemoryCallOnNonTrivialTypeCheck.h"
#include "../bugprone/ReservedIdentifierCheck.h"
#include "../bugprone/SignalHandlerCheck.h"
Expand All @@ -40,7 +41,6 @@
#include "../readability/EnumInitialValueCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "LimitedRandomnessCheck.h"
#include "ProperlySeededRandomGeneratorCheck.h"
#include "ThrownExceptionTypeCheck.h"

namespace {
Expand Down Expand Up @@ -271,7 +271,7 @@ class CERTModule : public ClangTidyModule {
"cert-mem57-cpp");
// MSC
CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp");
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
CheckFactories.registerCheck<bugprone::RandomGeneratorSeedCheck>(
"cert-msc51-cpp");
CheckFactories.registerCheck<bugprone::SignalHandlerCheck>(
"cert-msc54-cpp");
Expand Down Expand Up @@ -324,7 +324,7 @@ class CERTModule : public ClangTidyModule {
CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
"cert-msc24-c");
CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
CheckFactories.registerCheck<bugprone::RandomGeneratorSeedCheck>(
"cert-msc32-c");
CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
"cert-msc33-c");
Expand Down
1 change: 0 additions & 1 deletion clang-tools-extra/clang-tidy/cert/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyCERTModule STATIC
CERTTidyModule.cpp
LimitedRandomnessCheck.cpp
ProperlySeededRandomGeneratorCheck.cpp
ThrownExceptionTypeCheck.cpp

LINK_LIBS
Expand Down
10 changes: 10 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,16 @@ New check aliases
<clang-tidy/checks/bugprone/default-operator-new-on-overaligned-type>`
keeping initial check as an alias to the new one.

- Renamed :doc:`cert-msc32-c <clang-tidy/checks/cert/msc32-c>` to
:doc:`bugprone-random-generator-seed
<clang-tidy/checks/bugprone/random-generator-seed>`
keeping initial check as an alias to the new one.

- Renamed :doc:`cert-msc51-cpp <clang-tidy/checks/cert/msc51-cpp>` to
:doc:`bugprone-random-generator-seed
<clang-tidy/checks/bugprone/random-generator-seed>`
keeping initial check as an alias to the new one.

- Renamed :doc:`cert-oop57-cpp <clang-tidy/checks/cert/oop57-cpp>` to
:doc:`bugprone-raw-memory-call-on-non-trivial-type
<clang-tidy/checks/bugprone/raw-memory-call-on-non-trivial-type>`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
.. title:: clang-tidy - bugprone-random-generator-seed

bugprone-random-generator-seed
==============================

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.

Examples:

.. code-block:: c++

void foo() {
std::mt19937 engine1; // Diagnose, always generate the same sequence
std::mt19937 engine2(1); // Diagnose
engine1.seed(); // Diagnose
engine2.seed(1); // Diagnose

std::time_t t;
engine1.seed(std::time(&t)); // Diagnose, 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.
Default value is `time_t,std::time_t`.

References
----------

This check corresponds to the CERT C++ Coding Standard rules
`MSC51-CPP. Ensure your random number generator is properly seeded
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
`MSC32-C. Properly seed pseudorandom number generators
<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.
9 changes: 7 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/cert/msc32-c.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
.. title:: clang-tidy - cert-msc32-c
.. meta::
:http-equiv=refresh: 5;URL=../cert/msc51-cpp.html
:http-equiv=refresh: 5;URL=../bugprone/random-generator-seed.html

cert-msc32-c
============

The `cert-msc32-c` check is an alias, please see
:doc:`cert-msc51-cpp <../cert/msc51-cpp>` for more information.
:doc:`bugprone-random-generator-seed <../bugprone/random-generator-seed>`
for more information.

This check corresponds to the CERT C Coding Standard rule
`MSC32-C. Properly seed pseudorandom number generators
<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.
42 changes: 8 additions & 34 deletions clang-tools-extra/docs/clang-tidy/checks/cert/msc51-cpp.rst
Original file line number Diff line number Diff line change
@@ -1,40 +1,14 @@
.. title:: clang-tidy - cert-msc51-cpp
.. meta::
:http-equiv=refresh: 5;URL=../bugprone/random-generator-seed.html

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. Ensure your random number generator is properly seeded
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
`MSC32-C. Properly seed pseudorandom number generators
<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.

Examples:

.. code-block:: c++

void foo() {
std::mt19937 engine1; // Diagnose, always generate the same sequence
std::mt19937 engine2(1); // Diagnose
engine1.seed(); // Diagnose
engine2.seed(1); // Diagnose

std::time_t t;
engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user
The `cert-msc51-cpp` check is an alias, please see
:doc:`bugprone-random-generator-seed <../bugprone/random-generator-seed>`
for more information.

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.
Default value is `time_t,std::time_t`.
This check corresponds to the CERT C++ Coding Standard rule
`MSC51-CPP. Ensure your random number generator is properly seeded
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_.
5 changes: 3 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ Clang-Tidy Checks
:doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes"
:doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`,
:doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes"
:doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`,
:doc:`bugprone-raw-memory-call-on-non-trivial-type <bugprone/raw-memory-call-on-non-trivial-type>`,
:doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes"
:doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes"
Expand Down Expand Up @@ -181,7 +182,6 @@ Clang-Tidy Checks
:doc:`cert-err33-c <cert/err33-c>`,
:doc:`cert-err60-cpp <cert/err60-cpp>`,
:doc:`cert-msc50-cpp <cert/msc50-cpp>`,
:doc:`cert-msc51-cpp <cert/msc51-cpp>`,
:doc:`cert-oop58-cpp <cert/oop58-cpp>`,
:doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`,
:doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
Expand Down Expand Up @@ -458,8 +458,9 @@ Check aliases
:doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`bugprone-default-operator-new-on-overaligned-type <bugprone/default-operator-new-on-overaligned-type>`,
:doc:`cert-msc24-c <cert/msc24-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
:doc:`cert-msc30-c <cert/msc30-c>`, :doc:`cert-msc50-cpp <cert/msc50-cpp>`,
:doc:`cert-msc32-c <cert/msc32-c>`, :doc:`cert-msc51-cpp <cert/msc51-cpp>`,
:doc:`cert-msc32-c <cert/msc32-c>`, :doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`,
:doc:`cert-msc33-c <cert/msc33-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
:doc:`cert-msc51-cpp <cert/msc51-cpp>`, :doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`,
:doc:`cert-msc54-cpp <cert/msc54-cpp>`, :doc:`bugprone-signal-handler <bugprone/signal-handler>`,
:doc:`cert-oop11-cpp <cert/oop11-cpp>`, :doc:`performance-move-constructor-init <performance/move-constructor-init>`,
:doc:`cert-oop54-cpp <cert/oop54-cpp>`, :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
// RUN: %check_clang_tidy %s cert-msc32-c %t -- -config="{CheckOptions: {cert-msc32-c.DisallowedSeedTypes: 'some_type,time_t'}}" -- -std=c99
// RUN: %check_clang_tidy %s bugprone-random-generator-seed %t -- \
// RUN: -config="{CheckOptions: {bugprone-random-generator-seed.DisallowedSeedTypes: 'some_type,time_t'}}"

void srand(int seed);
typedef int time_t;
time_t time(time_t *t);

void f(void) {
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]
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [bugprone-random-generator-seed]

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]
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a constant value will generate a predictable sequence of values [bugprone-random-generator-seed]

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]
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: random number generator seeded with a disallowed source of seed value will generate a predictable sequence of values [bugprone-random-generator-seed]
}

void g(void) {
Expand Down
Loading