Skip to content

Commit fd994fd

Browse files
committed
[clang-tidy] Move 'cert-msc51-cpp', 'cert-msc32-c' checks outside of 'cert' module and give a proper name
Summary: This change addresses sub-issue #157296 of parent issue #157287, which aims to clean up the `cert` module by moving checks that don't directly map to CERT security standards to more appropriate modules. The checks `cert-msc51-cpp` and `cert-msc32-c` both implement the same logic to detect random number generators that are not properly seeded. This is a generic bug pattern rather than a CERT-specific security constraint, making it more suitable for the `bugprone` module. Closes: #157296
1 parent a0e222f commit fd994fd

File tree

12 files changed

+117
-103
lines changed

12 files changed

+117
-103
lines changed

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
#include "ParentVirtualCallCheck.h"
6565
#include "PointerArithmeticOnPolymorphicObjectCheck.h"
6666
#include "PosixReturnCheck.h"
67+
#include "ProperlySeededRandomGeneratorCheck.h"
6768
#include "RawMemoryCallOnNonTrivialTypeCheck.h"
6869
#include "RedundantBranchConditionCheck.h"
6970
#include "ReservedIdentifierCheck.h"
@@ -227,6 +228,8 @@ class BugproneModule : public ClangTidyModule {
227228
CheckFactories.registerCheck<ParentVirtualCallCheck>(
228229
"bugprone-parent-virtual-call");
229230
CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
231+
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
232+
"bugprone-random-generator-seed");
230233
CheckFactories.registerCheck<RawMemoryCallOnNonTrivialTypeCheck>(
231234
"bugprone-raw-memory-call-on-non-trivial-type");
232235
CheckFactories.registerCheck<ReservedIdentifierCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
6565
ParentVirtualCallCheck.cpp
6666
PointerArithmeticOnPolymorphicObjectCheck.cpp
6767
PosixReturnCheck.cpp
68+
ProperlySeededRandomGeneratorCheck.cpp
6869
RawMemoryCallOnNonTrivialTypeCheck.cpp
6970
RedundantBranchConditionCheck.cpp
7071
ReservedIdentifierCheck.cpp

clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h renamed to clang-tools-extra/clang-tidy/bugprone/ProperlySeededRandomGeneratorCheck.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,21 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
10-
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PROPERLY_SEEDED_RANDOM_GENERATOR_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PROPERLY_SEEDED_RANDOM_GENERATOR_H
1111

1212
#include "../ClangTidyCheck.h"
1313
#include <string>
1414

15-
namespace clang::tidy::cert {
15+
namespace clang::tidy::bugprone {
1616

1717
/// Random number generator must be seeded properly.
1818
///
1919
/// A random number generator initialized with default value or a
2020
/// constant expression is a security vulnerability.
2121
///
2222
/// For the user-facing documentation see:
23-
/// https://clang.llvm.org/extra/clang-tidy/checks/cert/msc51-cpp.html
23+
/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/random-generator-seed.html
2424
class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
2525
public:
2626
ProperlySeededRandomGeneratorCheck(StringRef Name, ClangTidyContext *Context);
@@ -37,6 +37,6 @@ class ProperlySeededRandomGeneratorCheck : public ClangTidyCheck {
3737
SmallVector<StringRef, 5> DisallowedSeedTypes;
3838
};
3939

40-
} // namespace clang::tidy::cert
40+
} // namespace clang::tidy::bugprone
4141

42-
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PROPERLYSEEDEDRANDOMGENERATORCHECK_H
42+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PROPERLY_SEEDED_RANDOM_GENERATOR_H

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "../bugprone/DefaultOperatorNewOnOveralignedTypeCheck.h"
1616
#include "../bugprone/FloatLoopCounterCheck.h"
1717
#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
18+
#include "../bugprone/ProperlySeededRandomGeneratorCheck.h"
1819
#include "../bugprone/RawMemoryCallOnNonTrivialTypeCheck.h"
1920
#include "../bugprone/ReservedIdentifierCheck.h"
2021
#include "../bugprone/SignalHandlerCheck.h"
@@ -41,6 +42,7 @@
4142
#include "../readability/UppercaseLiteralSuffixCheck.h"
4243
#include "LimitedRandomnessCheck.h"
4344
#include "ProperlySeededRandomGeneratorCheck.h"
45+
#include "MutatingCopyCheck.h"
4446
#include "ThrownExceptionTypeCheck.h"
4547

4648
namespace {
@@ -271,7 +273,7 @@ class CERTModule : public ClangTidyModule {
271273
"cert-mem57-cpp");
272274
// MSC
273275
CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp");
274-
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
276+
CheckFactories.registerCheck<bugprone::ProperlySeededRandomGeneratorCheck>(
275277
"cert-msc51-cpp");
276278
CheckFactories.registerCheck<bugprone::SignalHandlerCheck>(
277279
"cert-msc54-cpp");
@@ -324,7 +326,7 @@ class CERTModule : public ClangTidyModule {
324326
CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
325327
"cert-msc24-c");
326328
CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
327-
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
329+
CheckFactories.registerCheck<bugprone::ProperlySeededRandomGeneratorCheck>(
328330
"cert-msc32-c");
329331
CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
330332
"cert-msc33-c");

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ add_clang_library(clangTidyCERTModule STATIC
77
CERTTidyModule.cpp
88
LimitedRandomnessCheck.cpp
99
ProperlySeededRandomGeneratorCheck.cpp
10+
MutatingCopyCheck.cpp
1011
ThrownExceptionTypeCheck.cpp
1112

1213
LINK_LIBS
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
.. title:: clang-tidy - bugprone-random-generator-seed
2+
3+
bugprone-random-generator-seed
4+
==============================
5+
6+
This check flags all pseudo-random number engines, engine adaptor
7+
instantiations and ``srand()`` when initialized or seeded with default argument,
8+
constant expression or any user-configurable type. Pseudo-random number
9+
engines seeded with a predictable value may cause vulnerabilities e.g. in
10+
security protocols.
11+
This is a CERT security rule, see
12+
`MSC51-CPP. Ensure your random number generator is properly seeded
13+
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
14+
`MSC32-C. Properly seed pseudorandom number generators
15+
<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.
16+
17+
Examples:
18+
19+
.. code-block:: c++
20+
21+
void foo() {
22+
std::mt19937 engine1; // Diagnose, always generate the same sequence
23+
std::mt19937 engine2(1); // Diagnose
24+
engine1.seed(); // Diagnose
25+
engine2.seed(1); // Diagnose
26+
27+
std::time_t t;
28+
engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user
29+
30+
int x = atoi(argv[1]);
31+
std::mt19937 engine3(x); // Will not warn
32+
}
33+
34+
Options
35+
-------
36+
37+
.. option:: DisallowedSeedTypes
38+
39+
A comma-separated list of the type names which are disallowed.
40+
Default value is `time_t,std::time_t`.

clang-tools-extra/docs/clang-tidy/checks/cert/msc32-c.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ cert-msc32-c
66
============
77

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

clang-tools-extra/docs/clang-tidy/checks/cert/msc51-cpp.rst

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,5 @@
33
cert-msc51-cpp
44
==============
55

6-
This check flags all pseudo-random number engines, engine adaptor
7-
instantiations and ``srand()`` when initialized or seeded with default argument,
8-
constant expression or any user-configurable type. Pseudo-random number
9-
engines seeded with a predictable value may cause vulnerabilities e.g. in
10-
security protocols.
11-
This is a CERT security rule, see
12-
`MSC51-CPP. Ensure your random number generator is properly seeded
13-
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC51-CPP.+Ensure+your+random+number+generator+is+properly+seeded>`_ and
14-
`MSC32-C. Properly seed pseudorandom number generators
15-
<https://wiki.sei.cmu.edu/confluence/display/c/MSC32-C.+Properly+seed+pseudorandom+number+generators>`_.
16-
17-
Examples:
18-
19-
.. code-block:: c++
20-
21-
void foo() {
22-
std::mt19937 engine1; // Diagnose, always generate the same sequence
23-
std::mt19937 engine2(1); // Diagnose
24-
engine1.seed(); // Diagnose
25-
engine2.seed(1); // Diagnose
26-
27-
std::time_t t;
28-
engine1.seed(std::time(&t)); // Diagnose, system time might be controlled by user
29-
30-
int x = atoi(argv[1]);
31-
std::mt19937 engine3(x); // Will not warn
32-
}
33-
34-
Options
35-
-------
36-
37-
.. option:: DisallowedSeedTypes
38-
39-
A comma-separated list of the type names which are disallowed.
40-
Default value is `time_t,std::time_t`.
6+
The `cert-msc51-cpp` check is an alias, please see
7+
:doc:`bugprone-random-generator-seed <../bugprone/random-generator-seed>` for more information.

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ Clang-Tidy Checks
132132
:doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes"
133133
:doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`,
134134
:doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes"
135+
:doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`, "Yes"
135136
:doc:`bugprone-raw-memory-call-on-non-trivial-type <bugprone/raw-memory-call-on-non-trivial-type>`,
136137
:doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes"
137138
:doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes"
@@ -181,7 +182,6 @@ Clang-Tidy Checks
181182
:doc:`cert-err33-c <cert/err33-c>`,
182183
:doc:`cert-err60-cpp <cert/err60-cpp>`,
183184
:doc:`cert-msc50-cpp <cert/msc50-cpp>`,
184-
:doc:`cert-msc51-cpp <cert/msc51-cpp>`,
185185
:doc:`cert-oop58-cpp <cert/oop58-cpp>`,
186186
:doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`,
187187
:doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
@@ -458,7 +458,7 @@ Check aliases
458458
:doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`bugprone-default-operator-new-on-overaligned-type <bugprone/default-operator-new-on-overaligned-type>`,
459459
:doc:`cert-msc24-c <cert/msc24-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
460460
:doc:`cert-msc30-c <cert/msc30-c>`, :doc:`cert-msc50-cpp <cert/msc50-cpp>`,
461-
:doc:`cert-msc32-c <cert/msc32-c>`, :doc:`cert-msc51-cpp <cert/msc51-cpp>`,
461+
:doc:`cert-msc32-c <cert/msc32-c>`, :doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`,
462462
:doc:`cert-msc33-c <cert/msc33-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
463463
:doc:`cert-msc54-cpp <cert/msc54-cpp>`, :doc:`bugprone-signal-handler <bugprone/signal-handler>`,
464464
:doc:`cert-oop11-cpp <cert/oop11-cpp>`, :doc:`performance-move-constructor-init <performance/move-constructor-init>`,

0 commit comments

Comments
 (0)