Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -65,6 +65,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 @@ -230,6 +231,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 @@ -66,6 +66,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 @@ -16,6 +16,7 @@
#include "../bugprone/ExceptionCopyConstructorThrowsCheck.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 @@ -41,7 +42,6 @@
#include "../readability/EnumInitialValueCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "LimitedRandomnessCheck.h"
#include "ProperlySeededRandomGeneratorCheck.h"

namespace {

Expand Down Expand Up @@ -272,7 +272,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 @@ -325,7 +325,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

LINK_LIBS
clangTidy
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 @@ -283,6 +283,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 @@ -133,6 +133,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 @@ -183,7 +184,6 @@ Clang-Tidy Checks
:doc:`cert-err60-cpp <cert/err60-cpp>`,
:doc:`cert-flp30-c <cert/flp30-c>`,
: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 @@ -461,8 +461,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