From 84f007e427f3112442deeb7772f7cdcf2d694ce6 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:23:41 +0000 Subject: [PATCH 01/17] Contracts: add metadata. --- .../cpp/exclusions/c/Contracts.qll | 61 +++++++++++++++++ .../cpp/exclusions/c/RuleMetadata.qll | 3 + rule_packages/c/Contracts.json | 65 +++++++++++++++++++ rule_packages/cpp/Expressions.json | 1 + 4 files changed, 130 insertions(+) create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Contracts.qll create mode 100644 rule_packages/c/Contracts.json diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Contracts.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Contracts.qll new file mode 100644 index 0000000000..32a44a4355 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Contracts.qll @@ -0,0 +1,61 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype ContractsQuery = + TDoNotViolateInLineLinkageConstraintsQuery() or + TCheckMathLibraryFunctionParametersQuery() or + TFunctionErrorInformationUntestedQuery() + +predicate isContractsQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `doNotViolateInLineLinkageConstraints` query + ContractsPackage::doNotViolateInLineLinkageConstraintsQuery() and + queryId = + // `@id` for the `doNotViolateInLineLinkageConstraints` query + "c/cert/do-not-violate-in-line-linkage-constraints" and + ruleId = "MSC40-C" and + category = "rule" + or + query = + // `Query` instance for the `checkMathLibraryFunctionParameters` query + ContractsPackage::checkMathLibraryFunctionParametersQuery() and + queryId = + // `@id` for the `checkMathLibraryFunctionParameters` query + "c/misra/check-math-library-function-parameters" and + ruleId = "DIR-4-11" and + category = "required" + or + query = + // `Query` instance for the `functionErrorInformationUntested` query + ContractsPackage::functionErrorInformationUntestedQuery() and + queryId = + // `@id` for the `functionErrorInformationUntested` query + "c/misra/function-error-information-untested" and + ruleId = "DIR-4-7" and + category = "required" +} + +module ContractsPackage { + Query doNotViolateInLineLinkageConstraintsQuery() { + //autogenerate `Query` type + result = + // `Query` type for `doNotViolateInLineLinkageConstraints` query + TQueryC(TContractsPackageQuery(TDoNotViolateInLineLinkageConstraintsQuery())) + } + + Query checkMathLibraryFunctionParametersQuery() { + //autogenerate `Query` type + result = + // `Query` type for `checkMathLibraryFunctionParameters` query + TQueryC(TContractsPackageQuery(TCheckMathLibraryFunctionParametersQuery())) + } + + Query functionErrorInformationUntestedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `functionErrorInformationUntested` query + TQueryC(TContractsPackageQuery(TFunctionErrorInformationUntestedQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index c2771f4171..6425f27e28 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -9,6 +9,7 @@ import Concurrency2 import Concurrency3 import Concurrency4 import Concurrency5 +import Contracts import Contracts1 import Contracts2 import Contracts3 @@ -80,6 +81,7 @@ newtype TCQuery = TConcurrency3PackageQuery(Concurrency3Query q) or TConcurrency4PackageQuery(Concurrency4Query q) or TConcurrency5PackageQuery(Concurrency5Query q) or + TContractsPackageQuery(ContractsQuery q) or TContracts1PackageQuery(Contracts1Query q) or TContracts2PackageQuery(Contracts2Query q) or TContracts3PackageQuery(Contracts3Query q) or @@ -151,6 +153,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConcurrency3QueryMetadata(query, queryId, ruleId, category) or isConcurrency4QueryMetadata(query, queryId, ruleId, category) or isConcurrency5QueryMetadata(query, queryId, ruleId, category) or + isContractsQueryMetadata(query, queryId, ruleId, category) or isContracts1QueryMetadata(query, queryId, ruleId, category) or isContracts2QueryMetadata(query, queryId, ruleId, category) or isContracts3QueryMetadata(query, queryId, ruleId, category) or diff --git a/rule_packages/c/Contracts.json b/rule_packages/c/Contracts.json new file mode 100644 index 0000000000..e2239908f0 --- /dev/null +++ b/rule_packages/c/Contracts.json @@ -0,0 +1,65 @@ +{ + "CERT-C": { + "MSC40-C": { + "properties": { + "obligation": "rule" + }, + "queries": [ + { + "description": "Inlined external functions are prohibited by the language standard from defining modifiable static or thread storage objects, or referencing identifiers with internal linkage.", + "kind": "problem", + "name": "Do not violate inline linkage constraints", + "precision": "very-high", + "severity": "error", + "short_name": "DoNotViolateInLineLinkageConstraints", + "tags": [ + "correctness" + ] + } + ], + "title": "Do not violate constraints" + } + }, + "MISRA-C-2012": { + "DIR-4-11": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Range, domain or pole errors in math functions may return unexpected values, trigger floating-point exceptions or set unexpected error modes.", + "kind": "problem", + "name": "The validity of values passed to `math.h` library functions shall be checked", + "precision": "high", + "severity": "error", + "short_name": "CheckMathLibraryFunctionParameters", + "shared_implementation_short_name": "UncheckedRangeDomainPoleErrors", + "tags": [ + "correctness" + ] + } + ], + "title": "The validity of values passed to library functions shall be checked" + }, + "DIR-4-7": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "A function (whether it is part of the standard library, a third party library or a user defined function) may provide some means of indicating the occurrence of an error. This may be via a global error flag, a parametric error flag, a special return value or some other means. Whenever such a mechanism is provided by a function the calling program shall check for the indication of an error as soon as the function returns.", + "kind": "problem", + "name": "If a function generates error information, then that error information shall be tested", + "precision": "very-high", + "severity": "recommendation", + "short_name": "FunctionErrorInformationUntested", + "shared_implementation_short_name": "FunctionErroneousReturnValueNotTested", + "tags": [ + "maintainability" + ] + } + ], + "title": "If a function returns error information, then that error information shall be tested" + } + } +} \ No newline at end of file diff --git a/rule_packages/cpp/Expressions.json b/rule_packages/cpp/Expressions.json index c0a7b6bb0b..5668c78a0a 100644 --- a/rule_packages/cpp/Expressions.json +++ b/rule_packages/cpp/Expressions.json @@ -86,6 +86,7 @@ "precision": "very-high", "severity": "recommendation", "short_name": "FunctionErroneousReturnValueNotTested", + "shared_implementation_short_name": "FunctionErroneousReturnValueNotTested", "tags": [ "maintainability" ] From 52bdbd7f536c723267489f8822d8c81a99db96e2 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:33:15 +0000 Subject: [PATCH 02/17] MSC40-C: Add query for finding inline linkage constraints. Adds a query that finds cases where extern inlined functions reference internal linkage objects, or declare objects which are static or thread local. --- .../DoNotViolateInLineLinkageConstraints.md | 210 ++++++++++++++++++ .../DoNotViolateInLineLinkageConstraints.ql | 59 +++++ ...otViolateInLineLinkageConstraints.expected | 6 + ...DoNotViolateInLineLinkageConstraints.qlref | 1 + c/cert/test/rules/MSC40-C/test.c | 31 +++ 5 files changed, 307 insertions(+) create mode 100644 c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md create mode 100644 c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.ql create mode 100644 c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.expected create mode 100644 c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.qlref create mode 100644 c/cert/test/rules/MSC40-C/test.c diff --git a/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md new file mode 100644 index 0000000000..26545fb812 --- /dev/null +++ b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md @@ -0,0 +1,210 @@ +# MSC40-C: Do not violate inline linkage constraints + +This query implements the CERT-C rule MSC40-C: + +> Do not violate constraints + + +## Description + +According to the C Standard, 3.8 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO%2FIEC9899-2011)\], a constraint is a "restriction, either syntactic or semantic, by which the exposition of language elements is to be interpreted." Despite the similarity of the terms, a runtime constraint is not a kind of constraint. + +Violating any *shall* statement within a constraint clause in the C Standard requires an [implementation](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation) to issue a diagnostic message, the C Standard, 5.1.1.3 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO%2FIEC9899-2011)\] states + +> A conforming implementation shall produce at least one diagnostic message (identified in an implementation-defined manner) if a preprocessing translation unit or translation unit contains a violation of any syntax rule or constraint, even if the behavior is also explicitly specified as undefined or implementation-defined. Diagnostic messages need not be produced in other circumstances. + + +The C Standard further explains in a footnote + +> The intent is that an implementation should identify the nature of, and where possible localize, each violation. Of course, an implementation is free to produce any number of diagnostics as long as a valid program is still correctly translated. It may also successfully translate an invalid program. + + +Any constraint violation is a violation of this rule because it can result in an invalid program. + +## Noncompliant Code Example (Inline, Internal Linkage) + +The C Standard, 6.7.4, paragraph 3 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO%2FIEC9899-2011)\], states + +> An inline definition of a function with external linkage shall not contain a definition of a modifiable object with static or thread storage duration, and shall not contain a reference to an identifier with internal linkage. + + +The motivation behind this constraint lies in the semantics of inline definitions. Paragraph 7 of subclause 6.7.4 reads, in part: + +> An inline definition provides an alternative to an external definition, which a translator may use to implement any call to the function in the same translation unit. It is unspecified whether a call to the function uses the inline definition or the external definition. + + +That is, if a function has an external and inline definition, implementations are free to choose which definition to invoke (two distinct invocations of the function may call different definitions, one the external definition, the other the inline definition). Therefore, issues can arise when these definitions reference internally linked objects or mutable objects with static or thread storage duration. + +This noncompliant code example refers to a static variable with file scope and internal linkage from within an external inline function: + +```cpp +static int I = 12; +extern inline void func(int a) { + int b = a * I; + /* ... */ +} + +``` + +## Compliant Solution (Inline, Internal Linkage) + +This compliant solution omits the `static` qualifier; consequently, the variable `I` has external linkage by default: + +```cpp +int I = 12; +extern inline void func(int a) { + int b = a * I; + /* ... */ +} + +``` + +## Noncompliant Code Example (inline, Modifiable Static) + +This noncompliant code example defines a modifiable `static` variable within an `extern inline` function. + +```cpp +extern inline void func(void) { + static int I = 12; + /* Perform calculations which may modify I */ +} + +``` + +## Compliant Solution (Inline, Modifiable Static) + +This compliant solution removes the `static` keyword from the local variable definition. If the modifications to `I` must be retained between invocations of `func()`, it must be declared at file scope so that it will be defined with external linkage. + +```cpp +extern inline void func(void) { + int I = 12; + /* Perform calculations which may modify I */ +} +``` + +## Noncompliant Code Example (Inline, Modifiable static) + +This noncompliant code example includes two translation units: `file1.c` and `file2.c`. The first file, `file1.c`, defines a pseudorandom number generation function: + +```cpp +/* file1.c */ + +/* Externally linked definition of the function get_random() */ +extern unsigned int get_random(void) { + /* Initialize the seeds */ + static unsigned int m_z = 0xdeadbeef; + static unsigned int m_w = 0xbaddecaf; + + /* Compute the next pseudorandom value and update the seeds */ + m_z = 36969 * (m_z & 65535) + (m_z >> 16); + m_w = 18000 * (m_w & 65535) + (m_w >> 16); + return (m_z << 16) + m_w; +} + +``` +The left-shift operation in the last line may wrap, but this is permitted by exception INT30-C-EX3 to rule [INT30-C. Ensure that unsigned integer operations do not wrap](https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap). + +The second file, `file2.c`, defines an `inline` version of this function that references mutable `static` objects—namely, objects that maintain the state of the pseudorandom number generator. Separate invocations of the `get_random()` function can call different definitions, each operating on separate static objects, resulting in a faulty pseudorandom number generator. + +```cpp +/* file2.c */ + +/* Inline definition of get_random function */ +inline unsigned int get_random(void) { + /* + * Initialize the seeds + * Constraint violation: static duration storage referenced + * in non-static inline definition + */ + static unsigned int m_z = 0xdeadbeef; + static unsigned int m_w = 0xbaddecaf; + + /* Compute the next pseudorandom value and update the seeds */ + m_z = 36969 * (m_z & 65535) + (m_z >> 16); + m_w = 18000 * (m_w & 65535) + (m_w >> 16); + return (m_z << 16) + m_w; +} + +int main(void) { + unsigned int rand_no; + for (int ii = 0; ii < 100; ii++) { + /* + * Get a pseudorandom number. Implementation defined whether the + * inline definition in this file or the external definition + * in file2.c is called. + */ + rand_no = get_random(); + /* Use rand_no... */ + } + + /* ... */ + + /* + * Get another pseudorandom number. Behavior is + * implementation defined. + */ + rand_no = get_random(); + /* Use rand_no... */ + return 0; +} + +``` + +## Compliant Solution (Inline, Modifiable static) + +This compliant solution adds the `static` modifier to the `inline` function definition in `file2.c`, giving it internal linkage. All references to `get_random()` in `file.2.c` will now reference the internally linked definition. The first file, which was not changed, is not shown here. + +```cpp +/* file2.c */ + +/* Static inline definition of get_random function */ +static inline unsigned int get_random(void) { + /* + * Initialize the seeds. + * No more constraint violation; the inline function is now + * internally linked. + */ + static unsigned int m_z = 0xdeadbeef; + static unsigned int m_w = 0xbaddecaf; + + /* Compute the next pseudorandom value and update the seeds */ + m_z = 36969 * (m_z & 65535) + (m_z >> 16); + m_w = 18000 * (m_w & 65535) + (m_w >> 16); + return (m_z << 16) + m_w; +} + +int main(void) { + /* Generate pseudorandom numbers using get_random()... */ + return 0; +} + +``` + +## Risk Assessment + +Constraint violations are a broad category of error that can result in unexpected control flow and corrupted data. + +
Rule Severity Likelihood Remediation Cost Priority Level
MSC40-C Low Unlikely Medium P2 L3
+ + +## Automated Detection + +
Tool Version Checker Description
Astrée 23.04 alignas-extended assignment-to-non-modifiable-lvalue cast-pointer-void-arithmetic-implicit element-type-incomplete function-pointer-integer-cast-implicit function-return-type inappropriate-pointer-cast-implicit incompatible-function-pointer-conversion incompatible-object-pointer-conversion initializer-excess invalid-array-size non-constant-static-assert parameter-match-type pointer-integral-cast-implicit pointer-qualifier-cast-const-implicit pointer-qualifier-cast-volatile-implicit redeclaration return-empty return-non-empty static-assert type-compatibility type-compatibility-link type-specifier undeclared-parameter unnamed-parameter Partially checked
Helix QAC 2023.4 C0232, C0233, C0244, C0268, C0321, C0322, C0338, C0422, C0423, C0426, C0427, C0429, C0430, C0431, C0432, C0435, C0436, C0437, C0446, C0447, C0448, C0449, C0451, C0452, C0453, C0454, C0456, C0457, C0458, C0460, C0461, C0462, C0463, C0466, C0467, C0468, C0469, C0476, C0477, C0478, C0481, C0482, C0483, C0484, C0485, C0486, C0487, C0493, C0494, C0495, C0496, C0497, C0513, C0514, C0515, C0536, C0537, C0540, C0541, C0542, C0546, C0547, C0550, C0554, C0555, C0556, C0557, C0558, C0559, C0560, C0561, C0562, C0563, C0564, C0565, C0580, C0588, C0589, C0590, C0591, C0605, C0616, C0619, C0620, C0621, C0622, C0627, C0628, C0629, C0631, C0638, C0640, C0641, C0642, C0643, C0644, C0645, C0646, C0649, C0650, C0651, C0653, C0655, C0656, C0657, C0659, C0664, C0665, C0669, C0671, C0673, C0674, C0675, C0677, C0682, C0683, C0684, C0685, C0690, C0698, C0699, C0708, C0709, C0736, C0737, C0738, C0746, C0747, C0755, C0756, C0757, C0758, C0766, C0767, C0768, C0774, C0775, C0801, C0802, C0803, C0804, C0811, C0821, C0834, C0835, C0844, C0845, C0851, C0852, C0866, C0873, C0877, C0940, C0941, C0943, C0944, C1023, C1024, C1025, C1033, C1047, C1048, C1050, C1061, C1062, C3236, C3237, C3238, C3244 C++4122
Klocwork 2023.4 MISRA.FUNC.STATIC.REDECL
LDRA tool suite 9.7.1 21 S, 145 S, 323 S, 345 S, 387 S, 404 S, 481 S, 580 S, 612 S, 615 S, 646 S
Parasoft C/C++test 2023.1 CERT_C-MSC40-a An inline definition of a function with external linkage shall not contain definitions and uses of static objects
Polyspace Bug Finder CERT C: Rule MSC40-C Checks for inline constraint not respected (rule partially covered)
RuleChecker 23.04 alignas-extended assignment-to-non-modifiable-lvalue cast-pointer-void-arithmetic-implicit element-type-incomplete function-pointer-integer-cast-implicit function-return-type inappropriate-pointer-cast-implicit incompatible-function-pointer-conversion incompatible-object-pointer-conversion initializer-excess invalid-array-size non-constant-static-assert parameter-match-type pointer-integral-cast-implicit pointer-qualifier-cast-const-implicit pointer-qualifier-cast-volatile-implicit redeclaration return-empty return-non-empty static-assert type-compatibility type-compatibility-link type-specifier undeclared-parameter unnamed-parameter Partially checked
+ + +## Related Vulnerabilities + +Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+MSC40-C). + +## Bibliography + +
\[ ISO/IEC 9899:2011 \] 4, "Conformance" 5.1.1.3, "Diagnostics" 6.7.4, "Function Specifiers"
+ + +## Implementation notes + +None + +## References + +* CERT-C: [MSC40-C: Do not violate constraints](https://wiki.sei.cmu.edu/confluence/display/c) diff --git a/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.ql b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.ql new file mode 100644 index 0000000000..63dec179c6 --- /dev/null +++ b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.ql @@ -0,0 +1,59 @@ +/** + * @id c/cert/do-not-violate-in-line-linkage-constraints + * @name MSC40-C: Do not violate inline linkage constraints + * @description Inlined external functions are prohibited by the language standard from defining + * modifiable static or thread storage objects, or referencing identifiers with + * internal linkage. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/cert/id/msc40-c + * correctness + * external/cert/obligation/rule + */ + +import cpp +import codingstandards.c.cert +import codingstandards.cpp.Linkage + +/* + * This is C specific, because in C++ all extern function definitions must be identical. + * Only in C is it permitted for an extern function to be defined in multiple translation + * units with different implementations, when using the inline keyword. + */ + +from Element accessOrDecl, Variable v, Function f, string message +where + not isExcluded(f, ContractsPackage::doNotViolateInLineLinkageConstraintsQuery()) and + f.isInline() and + hasExternalLinkage(f) and + // Pre-emptively exclude compiler generated functions + not f.isCompilerGenerated() and + // This rule does not apply to C++, but exclude C++ specific cases anyway + not f instanceof MemberFunction and + not f.isFromUninstantiatedTemplate(_) and + ( + // There exists a modifiable local variable which is static or thread local + exists(LocalVariable lsv, string storageModifier | + lsv.isStatic() and storageModifier = "Static" + or + lsv.isThreadLocal() and storageModifier = "Thread-local" + | + lsv.getFunction() = f and + not lsv.isConst() and + accessOrDecl = lsv and + message = storageModifier + " local variable $@ declared" and + v = lsv + ) + or + // References an identifier with internal linkage + exists(GlobalOrNamespaceVariable gv | + accessOrDecl = v.getAnAccess() and + accessOrDecl.(VariableAccess).getEnclosingFunction() = f and + hasInternalLinkage(v) and + message = "Identifier $@ with internal linkage referenced" and + v = gv + ) + ) +select accessOrDecl, message + " in the extern inlined function $@.", v, v.getName(), f, + f.getQualifiedName() diff --git a/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.expected b/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.expected new file mode 100644 index 0000000000..f258d4adef --- /dev/null +++ b/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.expected @@ -0,0 +1,6 @@ +| test.c:6:14:6:14 | i | Static local variable $@ declared in the extern inlined function $@. | test.c:6:14:6:14 | i | i | test.c:5:20:5:24 | test1 | test1 | +| test.c:7:3:7:4 | g1 | Identifier $@ with internal linkage referenced in the extern inlined function $@. | test.c:1:12:1:13 | g1 | g1 | test.c:5:20:5:24 | test1 | test1 | +| test.c:9:3:9:4 | g3 | Identifier $@ with internal linkage referenced in the extern inlined function $@. | test.c:3:11:3:12 | g3 | g3 | test.c:5:20:5:24 | test1 | test1 | +| test.c:27:14:27:14 | i | Static local variable $@ declared in the extern inlined function $@. | test.c:27:14:27:14 | i | i | test.c:26:13:26:17 | test4 | test4 | +| test.c:28:3:28:4 | g1 | Identifier $@ with internal linkage referenced in the extern inlined function $@. | test.c:1:12:1:13 | g1 | g1 | test.c:26:13:26:17 | test4 | test4 | +| test.c:30:3:30:4 | g3 | Identifier $@ with internal linkage referenced in the extern inlined function $@. | test.c:3:11:3:12 | g3 | g3 | test.c:26:13:26:17 | test4 | test4 | diff --git a/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.qlref b/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.qlref new file mode 100644 index 0000000000..f14d4270cc --- /dev/null +++ b/c/cert/test/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.qlref @@ -0,0 +1 @@ +rules/MSC40-C/DoNotViolateInLineLinkageConstraints.ql \ No newline at end of file diff --git a/c/cert/test/rules/MSC40-C/test.c b/c/cert/test/rules/MSC40-C/test.c new file mode 100644 index 0000000000..3ca4afff4a --- /dev/null +++ b/c/cert/test/rules/MSC40-C/test.c @@ -0,0 +1,31 @@ +static int g1 = 0; +extern int g2 = 1; +const int g3 = 1; // defaults to internal linkage + +extern inline void test1() { + static int i = 0; // NON_COMPLIANT + g1++; // NON_COMPLIANT + g2++; // COMPLIANT + g3; // NON_COMPLIANT +} + +extern void test2() { + static int i = 0; // COMPLIANT + g1++; // COMPLIANT + g2++; // COMPLIANT + g3; // COMPLIANT +} + +void test3() { + static int i = 0; // COMPLIANT + g1++; // COMPLIANT + g2++; // COMPLIANT + g3; // COMPLIANT +} + +inline void test4() { + static int i = 0; // NON_COMPLIANT + g1++; // NON_COMPLIANT + g2++; // COMPLIANT + g3; // NON_COMPLIANT +} \ No newline at end of file From dcb32c1a017b50a1602a8d3e2e8afbf35ef0e534 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:44:43 +0000 Subject: [PATCH 03/17] DIR-4-11: Add query for checking math.h functions Add query which detects domain and pole errors for math.h functions. --- .../CheckMathLibraryFunctionParameters.ql | 22 +++++++++++++++++++ ...CheckMathLibraryFunctionParameters.testref | 1 + 2 files changed, 23 insertions(+) create mode 100644 c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql create mode 100644 c/misra/test/rules/DIR-4-11/CheckMathLibraryFunctionParameters.testref diff --git a/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql b/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql new file mode 100644 index 0000000000..6810784a0e --- /dev/null +++ b/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql @@ -0,0 +1,22 @@ +/** + * @id c/misra/check-math-library-function-parameters + * @name DIR-4-11: The validity of values passed to `math.h` library functions shall be checked + * @description Range, domain or pole errors in math functions may return unexpected values, trigger + * floating-point exceptions or set unexpected error modes. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/dir-4-11 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.uncheckedrangedomainpoleerrors.UncheckedRangeDomainPoleErrors + +class CheckMathLibraryFunctionParametersQuery extends UncheckedRangeDomainPoleErrorsSharedQuery { + CheckMathLibraryFunctionParametersQuery() { + this = ContractsPackage::checkMathLibraryFunctionParametersQuery() + } +} diff --git a/c/misra/test/rules/DIR-4-11/CheckMathLibraryFunctionParameters.testref b/c/misra/test/rules/DIR-4-11/CheckMathLibraryFunctionParameters.testref new file mode 100644 index 0000000000..50cf3fcb51 --- /dev/null +++ b/c/misra/test/rules/DIR-4-11/CheckMathLibraryFunctionParameters.testref @@ -0,0 +1 @@ +c/common/test/rules/uncheckedrangedomainpoleerrors/UncheckedRangeDomainPoleErrors.ql \ No newline at end of file From 377124004116573572197eabd39ceb905e8b5e30 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:47:55 +0000 Subject: [PATCH 04/17] DIR-4-7: Create shared query for unchecked return values Create a new shared query from the implementation of M0-3-2, which detects cases where error checking has not occurred after a call to a standard C library function. --- ...tionErroneousReturnValueNotTested.expected | 1 + .../FunctionErroneousReturnValueNotTested.ql | 4 ++ .../test.c | 0 .../FunctionErrorInformationUntested.ql | 26 ++++++++ .../FunctionErrorInformationUntested.testref | 1 + .../FunctionErroneousReturnValueNotTested.ql | 57 +++-------------- ...unctionErroneousReturnValueNotTested.qlref | 1 - .../FunctionErroneousReturnValueNotTested.qll | 62 +++++++++++++++++++ ...tionErroneousReturnValueNotTested.expected | 0 .../FunctionErroneousReturnValueNotTested.ql | 4 ++ .../test.cpp | 17 +++++ 11 files changed, 122 insertions(+), 51 deletions(-) create mode 100644 c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected create mode 100644 c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql rename cpp/autosar/test/rules/M0-3-2/test.cpp => c/common/test/rules/functionerroneousreturnvaluenottested/test.c (100%) create mode 100644 c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql create mode 100644 c/misra/test/rules/DIR-4-7/FunctionErrorInformationUntested.testref delete mode 100644 cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.qlref create mode 100644 cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll rename cpp/{autosar/test/rules/M0-3-2 => common/test/rules/functionerroneousreturnvaluenottested}/FunctionErroneousReturnValueNotTested.expected (100%) create mode 100644 cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql create mode 100644 cpp/common/test/rules/functionerroneousreturnvaluenottested/test.cpp diff --git a/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected new file mode 100644 index 0000000000..015f52348c --- /dev/null +++ b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected @@ -0,0 +1 @@ +| test.c:16:3:16:8 | call to remove | Return value is not tested for errors. | diff --git a/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql new file mode 100644 index 0000000000..12c2196efd --- /dev/null +++ b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql @@ -0,0 +1,4 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.functionerroneousreturnvaluenottested.FunctionErroneousReturnValueNotTested + +class TestFileQuery extends FunctionErroneousReturnValueNotTestedSharedQuery, TestQuery { } diff --git a/cpp/autosar/test/rules/M0-3-2/test.cpp b/c/common/test/rules/functionerroneousreturnvaluenottested/test.c similarity index 100% rename from cpp/autosar/test/rules/M0-3-2/test.cpp rename to c/common/test/rules/functionerroneousreturnvaluenottested/test.c diff --git a/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql new file mode 100644 index 0000000000..b827e101e3 --- /dev/null +++ b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql @@ -0,0 +1,26 @@ +/** + * @id c/misra/function-error-information-untested + * @name DIR-4-7: If a function generates error information, then that error information shall be tested + * @description A function (whether it is part of the standard library, a third party library or a + * user defined function) may provide some means of indicating the occurrence of an + * error. This may be via a global error flag, a parametric error flag, a special + * return value or some other means. Whenever such a mechanism is provided by a + * function the calling program shall check for the indication of an error as soon as + * the function returns. + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags external/misra/id/dir-4-7 + * maintainability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.functionerroneousreturnvaluenottested.FunctionErroneousReturnValueNotTested + +class FunctionErrorInformationUntestedQuery extends FunctionErroneousReturnValueNotTestedSharedQuery { + FunctionErrorInformationUntestedQuery() { + this = ContractsPackage::functionErrorInformationUntestedQuery() + } +} diff --git a/c/misra/test/rules/DIR-4-7/FunctionErrorInformationUntested.testref b/c/misra/test/rules/DIR-4-7/FunctionErrorInformationUntested.testref new file mode 100644 index 0000000000..51bd5fbefb --- /dev/null +++ b/c/misra/test/rules/DIR-4-7/FunctionErrorInformationUntested.testref @@ -0,0 +1 @@ +c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql \ No newline at end of file diff --git a/cpp/autosar/src/rules/M0-3-2/FunctionErroneousReturnValueNotTested.ql b/cpp/autosar/src/rules/M0-3-2/FunctionErroneousReturnValueNotTested.ql index aee4e40838..77e6278960 100644 --- a/cpp/autosar/src/rules/M0-3-2/FunctionErroneousReturnValueNotTested.ql +++ b/cpp/autosar/src/rules/M0-3-2/FunctionErroneousReturnValueNotTested.ql @@ -19,54 +19,11 @@ import cpp import codingstandards.cpp.autosar -import codingstandards.cpp.dataflow.DataFlow -import semmle.code.cpp.controlflow.Guards +import codingstandards.cpp.rules.functionerroneousreturnvaluenottested.FunctionErroneousReturnValueNotTested -from FunctionCall fc -where - not isExcluded(fc, ExpressionsPackage::functionErroneousReturnValueNotTestedQuery()) and - fc.getTarget() - .hasGlobalOrStdName([ - // fcntl.h - "open", "openat", "fcntl", "creat", - // locale.h - "setlocale", - // stdlib.h - "system", "getenv", "getenv_s", - // signal.h - "signal", "raise", - // setjmp.h - "setjmp", - // stdio.h - "fopen", "fopen_s", "freopen", "freopen_s", "fclose", "fcloseall", "fflush", "setvbuf", - "fgetc", "getc", "fgets", "fputc", "getchar", "gets", "gets_s", "putchar", "puts", - "ungetc", "scanf", "fscanf", "sscanf", "scanf_s", "fscanf_s", "sscanf_s", "vscanf", - "vfscanf", "vsscanf", "vscanf_s", "vfscanf_s", "vsscanf_s", "printf", "fprintf", - "sprintf", "snprintf", "printf_s", "fprintf_s", "sprintf_s", "snprintf_s", "vprintf", - "vfprintf", "vsprintf", "vsnprintf", "vprintf_s", "vfprintf_s", "vsprintf_s", - "vsnprintf_s", "ftell", "fgetpos", "fseek", "fsetpos", "remove", "rename", "tmpfile", - "tmpfile_s", "tmpnam", "tmpnam_s", - // string.h - "strcpy_s", "strncpy_s", "strcat_s", "strncat_s", "memset_s", "memcpy_s", "memmove_s", - "strerror_s", - // threads.h - "thrd_create", "thrd_sleep", "thrd_detach", "thrd_join", "mtx_init", "mtx_lock", - "mtx_timedlock", "mtx_trylock", "mtx_unlock", "cnd_init", "cnd_signal", "cnd_broadcast", - "cnd_wait", "cnd_timedwait", "tss_create", "tss_get", "tss_set", - // time.h - "time", "clock", "timespec_get", "asctime_s", "ctime_s", "gmtime", "gmtime_s", - "localtime", "localtime_s", - // unistd.h - "write", "read", "close", "unlink", - // wchar.h - "fgetwc", "getwc", "fgetws", "fputwc", "putwc", "fputws", "getwchar", "putwchar", - "ungetwc", "wscanf", "fwscanf", "swscanf", "wscanf_s", "fwscanf_s", "swscanf_s", - "vwscanf", "vfwscanf", "vswscanf", "vwscanf_s", "vfwscanf_s", "vswscanf_s", "wprintf", - "fwprintf", "swprintf", "wprintf_s", "fwprintf_s", "swprintf_s", "snwprintf_s", - "vwprintf", "vfwprintf", "vswprintf", "vwprintf_s", "vfwprintf_s", "vswprintf_s", - "vsnwprintf_s" - ]) and - forall(GuardCondition gc | - not DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.getAChild*())) - ) -select fc, "Return value is not tested for errors." +class FunctionErrorInformationUntestedQuery extends FunctionErroneousReturnValueNotTestedSharedQuery +{ + FunctionErrorInformationUntestedQuery() { + this = ExpressionsPackage::functionErroneousReturnValueNotTestedQuery() + } +} diff --git a/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.qlref b/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.qlref deleted file mode 100644 index 3cfea1dc31..0000000000 --- a/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/M0-3-2/FunctionErroneousReturnValueNotTested.ql \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll new file mode 100644 index 0000000000..fe4f788847 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll @@ -0,0 +1,62 @@ +/** + * Provides a library which includes a `problems` predicate for reporting unchecked error values. + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.dataflow.DataFlow +import semmle.code.cpp.controlflow.Guards +import codingstandards.cpp.Exclusions + +abstract class FunctionErroneousReturnValueNotTestedSharedQuery extends Query { } + +Query getQuery() { result instanceof FunctionErroneousReturnValueNotTestedSharedQuery } + +query predicate problems(FunctionCall fc, string message) { + not isExcluded(fc, getQuery()) and + fc.getTarget() + .hasGlobalOrStdName([ + // fcntl.h + "open", "openat", "fcntl", "creat", + // locale.h + "setlocale", + // stdlib.h + "system", "getenv", "getenv_s", + // signal.h + "signal", "raise", + // setjmp.h + "setjmp", + // stdio.h + "fopen", "fopen_s", "freopen", "freopen_s", "fclose", "fcloseall", "fflush", "setvbuf", + "fgetc", "getc", "fgets", "fputc", "getchar", "gets", "gets_s", "putchar", "puts", + "ungetc", "scanf", "fscanf", "sscanf", "scanf_s", "fscanf_s", "sscanf_s", "vscanf", + "vfscanf", "vsscanf", "vscanf_s", "vfscanf_s", "vsscanf_s", "printf", "fprintf", + "sprintf", "snprintf", "printf_s", "fprintf_s", "sprintf_s", "snprintf_s", "vprintf", + "vfprintf", "vsprintf", "vsnprintf", "vprintf_s", "vfprintf_s", "vsprintf_s", + "vsnprintf_s", "ftell", "fgetpos", "fseek", "fsetpos", "remove", "rename", "tmpfile", + "tmpfile_s", "tmpnam", "tmpnam_s", + // string.h + "strcpy_s", "strncpy_s", "strcat_s", "strncat_s", "memset_s", "memcpy_s", "memmove_s", + "strerror_s", + // threads.h + "thrd_create", "thrd_sleep", "thrd_detach", "thrd_join", "mtx_init", "mtx_lock", + "mtx_timedlock", "mtx_trylock", "mtx_unlock", "cnd_init", "cnd_signal", "cnd_broadcast", + "cnd_wait", "cnd_timedwait", "tss_create", "tss_get", "tss_set", + // time.h + "time", "clock", "timespec_get", "asctime_s", "ctime_s", "gmtime", "gmtime_s", + "localtime", "localtime_s", + // unistd.h + "write", "read", "close", "unlink", + // wchar.h + "fgetwc", "getwc", "fgetws", "fputwc", "putwc", "fputws", "getwchar", "putwchar", + "ungetwc", "wscanf", "fwscanf", "swscanf", "wscanf_s", "fwscanf_s", "swscanf_s", + "vwscanf", "vfwscanf", "vswscanf", "vwscanf_s", "vfwscanf_s", "vswscanf_s", "wprintf", + "fwprintf", "swprintf", "wprintf_s", "fwprintf_s", "swprintf_s", "snwprintf_s", + "vwprintf", "vfwprintf", "vswprintf", "vwprintf_s", "vfwprintf_s", "vswprintf_s", + "vsnwprintf_s" + ]) and + forall(GuardCondition gc | + not DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.getAChild*())) + ) and + message = "Return value is not tested for errors." +} diff --git a/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.expected b/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected similarity index 100% rename from cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.expected rename to cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected diff --git a/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql b/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql new file mode 100644 index 0000000000..12c2196efd --- /dev/null +++ b/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql @@ -0,0 +1,4 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.functionerroneousreturnvaluenottested.FunctionErroneousReturnValueNotTested + +class TestFileQuery extends FunctionErroneousReturnValueNotTestedSharedQuery, TestQuery { } diff --git a/cpp/common/test/rules/functionerroneousreturnvaluenottested/test.cpp b/cpp/common/test/rules/functionerroneousreturnvaluenottested/test.cpp new file mode 100644 index 0000000000..08e2f23dec --- /dev/null +++ b/cpp/common/test/rules/functionerroneousreturnvaluenottested/test.cpp @@ -0,0 +1,17 @@ +#include + +void test_compliant() { + // Return value is passed to an lvalue and then tested. + FILE *fh = fopen("/etc/foo", "r"); + if (!fh) { // COMPLIANT + return; + } + + // Return value is tested immediately as an rvalue. + if (fclose(fh)) // COMPLIANT + return; +} + +void test_noncompliant() { + remove("/bin/bash"); // NON_COMPLIANT +} \ No newline at end of file From 81200896b07034ad5120918e91df6295f2d9feb0 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:53:23 +0000 Subject: [PATCH 05/17] FunctionErroneousReturnValueNotTested rewritten for clarity --- .../FunctionErroneousReturnValueNotTested.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll index fe4f788847..5cd98c05d6 100644 --- a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll +++ b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll @@ -55,8 +55,8 @@ query predicate problems(FunctionCall fc, string message) { "vwprintf", "vfwprintf", "vswprintf", "vwprintf_s", "vfwprintf_s", "vswprintf_s", "vsnwprintf_s" ]) and - forall(GuardCondition gc | - not DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.getAChild*())) + not exists(GuardCondition gc | + DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.getAChild*())) ) and message = "Return value is not tested for errors." } From 7e5ef5f8306fee09522415cfda8cf708d8a32ea0 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 09:55:42 +0000 Subject: [PATCH 06/17] Add name of function to message. --- .../FunctionErroneousReturnValueNotTested.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll index 5cd98c05d6..dd2f7d75e0 100644 --- a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll +++ b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll @@ -58,5 +58,5 @@ query predicate problems(FunctionCall fc, string message) { not exists(GuardCondition gc | DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.getAChild*())) ) and - message = "Return value is not tested for errors." + message = "Return value from " + fc.getTarget().getName() + " is not tested for errors." } From a19855bd20ace838d2f7748f45de341e5e7b97ef Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 10:02:11 +0000 Subject: [PATCH 07/17] Add implementation scope properties Specify the scope of each of the newly supported rules. --- rule_packages/c/Contracts.json | 15 ++++++++++++--- rule_packages/c/FloatingTypes.json | 5 ++++- rule_packages/cpp/Expressions.json | 5 ++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/rule_packages/c/Contracts.json b/rule_packages/c/Contracts.json index e2239908f0..735e84d9da 100644 --- a/rule_packages/c/Contracts.json +++ b/rule_packages/c/Contracts.json @@ -14,7 +14,10 @@ "short_name": "DoNotViolateInLineLinkageConstraints", "tags": [ "correctness" - ] + ], + "implementation_scope": { + "description": "This query only considers the constraints related to inline extern functions." + } } ], "title": "Do not violate constraints" @@ -36,7 +39,10 @@ "shared_implementation_short_name": "UncheckedRangeDomainPoleErrors", "tags": [ "correctness" - ] + ], + "implementation_scope": { + "description": "This query identifies possible domain, pole and range errors on a selection of C standard library fuctions from math.h." + } } ], "title": "The validity of values passed to library functions shall be checked" @@ -56,7 +62,10 @@ "shared_implementation_short_name": "FunctionErroneousReturnValueNotTested", "tags": [ "maintainability" - ] + ], + "implementation_scope": { + "description": "This query enforces checking on some C standard library functions that may return error codes." + } } ], "title": "If a function returns error information, then that error information shall be tested" diff --git a/rule_packages/c/FloatingTypes.json b/rule_packages/c/FloatingTypes.json index 1dfd663597..7df2298ad1 100644 --- a/rule_packages/c/FloatingTypes.json +++ b/rule_packages/c/FloatingTypes.json @@ -15,7 +15,10 @@ "shared_implementation_short_name": "UncheckedRangeDomainPoleErrors", "tags": [ "correctness" - ] + ], + "implementation_scope": { + "description": "This query identifies possible domain, pole and range errors on a selection of C standard library fuctions from math.h." + } } ], "title": "Prevent or detect domain and range errors in math functions" diff --git a/rule_packages/cpp/Expressions.json b/rule_packages/cpp/Expressions.json index 5668c78a0a..935c3fa6f1 100644 --- a/rule_packages/cpp/Expressions.json +++ b/rule_packages/cpp/Expressions.json @@ -89,7 +89,10 @@ "shared_implementation_short_name": "FunctionErroneousReturnValueNotTested", "tags": [ "maintainability" - ] + ], + "implementation_scope": { + "description": "The query enforces checking on some C standard library functions that may return error codes." + } } ], "title": "If a function generates error information, then that error information shall be tested." From 9b7102e149ad57a8b45f6b09b5df452d731f1f2c Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 10:03:21 +0000 Subject: [PATCH 08/17] Add change note. --- change_notes/2024-01-30-m0-3-2.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 change_notes/2024-01-30-m0-3-2.md diff --git a/change_notes/2024-01-30-m0-3-2.md b/change_notes/2024-01-30-m0-3-2.md new file mode 100644 index 0000000000..b074f6b2b1 --- /dev/null +++ b/change_notes/2024-01-30-m0-3-2.md @@ -0,0 +1 @@ + * `M0-3-2` - the alert messages now include the name of the called function. \ No newline at end of file From 78d5e793b66136c9ea7eec6c3da011d0af335011 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 11:33:02 +0000 Subject: [PATCH 09/17] Add missing file. --- .../rules/M0-3-2/FunctionErroneousReturnValueNotTested.testref | 1 + 1 file changed, 1 insertion(+) create mode 100644 cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.testref diff --git a/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.testref b/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.testref new file mode 100644 index 0000000000..50847523ce --- /dev/null +++ b/cpp/autosar/test/rules/M0-3-2/FunctionErroneousReturnValueNotTested.testref @@ -0,0 +1 @@ +cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.ql \ No newline at end of file From 9776de1211801f333c827630352daf9bef931455 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 12:58:50 +0000 Subject: [PATCH 10/17] Update expected result files. --- .../FunctionErroneousReturnValueNotTested.expected | 2 +- .../FunctionErroneousReturnValueNotTested.expected | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected index 015f52348c..dc72201a8a 100644 --- a/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected +++ b/c/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected @@ -1 +1 @@ -| test.c:16:3:16:8 | call to remove | Return value is not tested for errors. | +| test.c:16:3:16:8 | call to remove | Return value from remove is not tested for errors. | diff --git a/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected b/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected index 76cbcebed0..2f681c9210 100644 --- a/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected +++ b/cpp/common/test/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.expected @@ -1 +1 @@ -| test.cpp:16:3:16:8 | call to remove | Return value is not tested for errors. | +| test.cpp:16:3:16:8 | call to remove | Return value from remove is not tested for errors. | From af9226208ee43cb49f0940a36972be3061ca7c01 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 13:00:51 +0000 Subject: [PATCH 11/17] Update rules.csv to exclude unimplemented contracts rules. --- rules.csv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules.csv b/rules.csv index 20af9fbc01..9f4afac4fa 100644 --- a/rules.csv +++ b/rules.csv @@ -614,8 +614,8 @@ c,MISRA-C-2012,DIR-4-9,Yes,Advisory,,,A function should be used in preference to c,MISRA-C-2012,DIR-4-10,Yes,Required,,,Precautions shall be taken in order to prevent the contents of a header file being included more than once,M16-2-3,Preprocessor2,Medium, c,MISRA-C-2012,DIR-4-11,Yes,Required,,,The validity of values passed to library functions shall be checked,,Contracts,Hard, c,MISRA-C-2012,DIR-4-12,Yes,Required,,,Dynamic memory allocation shall not be used,,Banned,Medium, -c,MISRA-C-2012,DIR-4-13,Yes,Advisory,,,Functions which are designed to provide operations on a resource should be called in an appropriate sequence,,Contracts,Hard, -c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts,Hard, +c,MISRA-C-2012,DIR-4-13,No,Advisory,,,Functions which are designed to provide operations on a resource should be called in an appropriate sequence,,,,Rule 22.1, 22.2 and 22.6 cover aspects of this rule. In other cases this is a design issue and needs to be checked manually. +c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts9,Hard,This is supported by CodeQLs default C security queries. c,MISRA-C-2012,RULE-1-1,No,Required,,,"The program shall contain no violations of the standard C syntax and constraints, and shall not exceed the implementation's translation limits",,,Easy,"This should be checked via the compiler output, rather than CodeQL, which adds unnecessary steps." c,MISRA-C-2012,RULE-1-2,Yes,Advisory,,,Language extensions should not be used,,Language3,Hard, c,MISRA-C-2012,RULE-1-3,Yes,Required,,,There shall be no occurrence of undefined or critical unspecified behaviour,,Language3,Hard, From 3d2cd94fd92681127dbb38fa0833150a2536eb00 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 13:13:14 +0000 Subject: [PATCH 12/17] Update documentation. --- c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md | 2 +- .../src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md b/c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md index d6427b9081..ca24a02498 100644 --- a/c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md +++ b/c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md @@ -345,7 +345,7 @@ Independent( INT34-C, FLP32-C, INT33-C) CWE-682 = Union( FLP32-C, list) where li ## Implementation notes -None +This query identifies possible domain, pole and range errors on a selection of C standard library fuctions from math.h. ## References diff --git a/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md index 26545fb812..f767c91baf 100644 --- a/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md +++ b/c/cert/src/rules/MSC40-C/DoNotViolateInLineLinkageConstraints.md @@ -203,7 +203,7 @@ Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+D ## Implementation notes -None +This query only considers the constraints related to inline extern functions. ## References From bccdb93c2b7b0e2dcbe82feb134c0c0423ae9d81 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 13:13:59 +0000 Subject: [PATCH 13/17] Fix formatting --- c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql index b827e101e3..63236d422d 100644 --- a/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql +++ b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql @@ -19,7 +19,8 @@ import cpp import codingstandards.c.misra import codingstandards.cpp.rules.functionerroneousreturnvaluenottested.FunctionErroneousReturnValueNotTested -class FunctionErrorInformationUntestedQuery extends FunctionErroneousReturnValueNotTestedSharedQuery { +class FunctionErrorInformationUntestedQuery extends FunctionErroneousReturnValueNotTestedSharedQuery +{ FunctionErrorInformationUntestedQuery() { this = ContractsPackage::functionErrorInformationUntestedQuery() } From 337604e56719a36e6098e8b8b41114168e3ab39a Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 30 Jan 2024 13:14:37 +0000 Subject: [PATCH 14/17] Fix test formatting --- c/cert/test/rules/MSC40-C/test.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/c/cert/test/rules/MSC40-C/test.c b/c/cert/test/rules/MSC40-C/test.c index 3ca4afff4a..d892935d41 100644 --- a/c/cert/test/rules/MSC40-C/test.c +++ b/c/cert/test/rules/MSC40-C/test.c @@ -4,28 +4,28 @@ const int g3 = 1; // defaults to internal linkage extern inline void test1() { static int i = 0; // NON_COMPLIANT - g1++; // NON_COMPLIANT - g2++; // COMPLIANT - g3; // NON_COMPLIANT + g1++; // NON_COMPLIANT + g2++; // COMPLIANT + g3; // NON_COMPLIANT } extern void test2() { static int i = 0; // COMPLIANT - g1++; // COMPLIANT - g2++; // COMPLIANT - g3; // COMPLIANT + g1++; // COMPLIANT + g2++; // COMPLIANT + g3; // COMPLIANT } void test3() { static int i = 0; // COMPLIANT - g1++; // COMPLIANT - g2++; // COMPLIANT - g3; // COMPLIANT + g1++; // COMPLIANT + g2++; // COMPLIANT + g3; // COMPLIANT } inline void test4() { static int i = 0; // NON_COMPLIANT - g1++; // NON_COMPLIANT - g2++; // COMPLIANT - g3; // NON_COMPLIANT + g1++; // NON_COMPLIANT + g2++; // COMPLIANT + g3; // NON_COMPLIANT } \ No newline at end of file From 6b8ba85dea62ff4ebac192b15db278e51cb431c0 Mon Sep 17 00:00:00 2001 From: Luke Cartey <5377966+lcartey@users.noreply.github.com> Date: Sun, 5 Jan 2025 23:05:14 +0000 Subject: [PATCH 15/17] Update rules.csv Use Contracts8 as next entry --- rules.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules.csv b/rules.csv index 8391149b9f..a7f6c1f0db 100644 --- a/rules.csv +++ b/rules.csv @@ -615,7 +615,7 @@ c,MISRA-C-2012,DIR-4-10,Yes,Required,,,Precautions shall be taken in order to pr c,MISRA-C-2012,DIR-4-11,Yes,Required,,,The validity of values passed to library functions shall be checked,,Contracts,Hard, c,MISRA-C-2012,DIR-4-12,Yes,Required,,,Dynamic memory allocation shall not be used,,Banned,Medium, c,MISRA-C-2012,DIR-4-13,No,Advisory,,,Functions which are designed to provide operations on a resource should be called in an appropriate sequence,,,,"Rule 22.1, 22.2 and 22.6 cover aspects of this rule. In other cases this is a design issue and needs to be checked manually." -c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts9,Hard,This is supported by CodeQLs default C security queries. +c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts8,Hard,This is supported by CodeQLs default C security queries. c,MISRA-C-2012,RULE-1-1,No,Required,,,"The program shall contain no violations of the standard C syntax and constraints, and shall not exceed the implementation's translation limits",,,Easy,"This should be checked via the compiler output, rather than CodeQL, which adds unnecessary steps." c,MISRA-C-2012,RULE-1-2,Yes,Advisory,,,Language extensions should not be used,,Language3,Hard, c,MISRA-C-2012,RULE-1-3,Yes,Required,,,There shall be no occurrence of undefined or critical unspecified behaviour,,Language3,Hard, From 99a310642c211ff13204cbbe624ea999b6d545a0 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Sun, 12 Jan 2025 22:08:19 +0000 Subject: [PATCH 16/17] Contracts: Add MISRA C 2012 tags --- rule_packages/c/Contracts.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rule_packages/c/Contracts.json b/rule_packages/c/Contracts.json index 735e84d9da..40bf3d8b0b 100644 --- a/rule_packages/c/Contracts.json +++ b/rule_packages/c/Contracts.json @@ -38,7 +38,8 @@ "short_name": "CheckMathLibraryFunctionParameters", "shared_implementation_short_name": "UncheckedRangeDomainPoleErrors", "tags": [ - "correctness" + "correctness", + "external/misra/c/2012/third-edition-first-revision" ], "implementation_scope": { "description": "This query identifies possible domain, pole and range errors on a selection of C standard library fuctions from math.h." @@ -61,7 +62,8 @@ "short_name": "FunctionErrorInformationUntested", "shared_implementation_short_name": "FunctionErroneousReturnValueNotTested", "tags": [ - "maintainability" + "maintainability", + "external/misra/c/2012/third-edition-first-revision" ], "implementation_scope": { "description": "This query enforces checking on some C standard library functions that may return error codes." From 82440695528fce9e12846cb92298572cd7aa1fdd Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Sun, 12 Jan 2025 22:15:43 +0000 Subject: [PATCH 17/17] Regenerate package files --- c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql | 1 + c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql | 1 + 2 files changed, 2 insertions(+) diff --git a/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql b/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql index 6810784a0e..4011b210f8 100644 --- a/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql +++ b/c/misra/src/rules/DIR-4-11/CheckMathLibraryFunctionParameters.ql @@ -8,6 +8,7 @@ * @problem.severity error * @tags external/misra/id/dir-4-11 * correctness + * external/misra/c/2012/third-edition-first-revision * external/misra/obligation/required */ diff --git a/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql index 63236d422d..0c0a3d7b1a 100644 --- a/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql +++ b/c/misra/src/rules/DIR-4-7/FunctionErrorInformationUntested.ql @@ -12,6 +12,7 @@ * @problem.severity recommendation * @tags external/misra/id/dir-4-7 * maintainability + * external/misra/c/2012/third-edition-first-revision * external/misra/obligation/required */