diff --git a/c/misra/src/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql b/c/misra/src/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql new file mode 100644 index 0000000000..c95612b7ba --- /dev/null +++ b/c/misra/src/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql @@ -0,0 +1,33 @@ +/** + * @id c/misra/function-addresses-should-address-operator + * @name RULE-17-12: A function identifier should only be called with a parenthesized parameter list or used with a & + * @description A function identifier should only be called with a parenthesized parameter list or + * used with a & (address-of). + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-17-12 + * readability + * external/misra/c/2012/amendment3 + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.c.misra + +predicate isImplicitlyAddressed(FunctionAccess access) { + not access.getParent() instanceof AddressOfExpr and + // Note: the following *seems* to only exist in c++ codebases, for instance, + // when calling a member. In c, this syntax should always extract as a + // [FunctionCall] rather than a [ExprCall] of a [FunctionAccess]. Still, this + // is a good pattern to be defensive against. + not exists(ExprCall call | call.getExpr() = access) +} + +from FunctionAccess funcAccess +where + not isExcluded(funcAccess, FunctionTypesPackage::functionAddressesShouldAddressOperatorQuery()) and + isImplicitlyAddressed(funcAccess) +select funcAccess, + "The address of function " + funcAccess.getTarget().getName() + + " is taken without the & operator." diff --git a/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.expected b/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.expected new file mode 100644 index 0000000000..5a37cbd97e --- /dev/null +++ b/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.expected @@ -0,0 +1,13 @@ +| test.c:14:25:14:29 | func2 | The address of function func2 is taken without the & operator. | +| test.c:15:25:15:29 | func3 | The address of function func3 is taken without the & operator. | +| test.c:21:12:21:16 | func1 | The address of function func1 is taken without the & operator. | +| test.c:38:3:38:7 | func1 | The address of function func1 is taken without the & operator. | +| test.c:39:3:39:7 | func2 | The address of function func2 is taken without the & operator. | +| test.c:57:13:57:17 | func1 | The address of function func1 is taken without the & operator. | +| test.c:58:21:58:25 | func2 | The address of function func2 is taken without the & operator. | +| test.c:59:13:59:17 | func1 | The address of function func1 is taken without the & operator. | +| test.c:59:20:59:24 | func2 | The address of function func2 is taken without the & operator. | +| test.c:67:11:67:15 | func1 | The address of function func1 is taken without the & operator. | +| test.c:68:12:68:16 | func1 | The address of function func1 is taken without the & operator. | +| test.c:69:12:69:16 | func1 | The address of function func1 is taken without the & operator. | +| test.c:71:18:71:22 | func1 | The address of function func1 is taken without the & operator. | diff --git a/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.qlref b/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.qlref new file mode 100644 index 0000000000..f0a4753620 --- /dev/null +++ b/c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.qlref @@ -0,0 +1 @@ +rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-12/test.c b/c/misra/test/rules/RULE-17-12/test.c new file mode 100644 index 0000000000..5ab5a4984d --- /dev/null +++ b/c/misra/test/rules/RULE-17-12/test.c @@ -0,0 +1,107 @@ +void func1() {} +void func2(int x, char *y) {} + +typedef struct { +} s; + +int func3() { return 0; } + +typedef void (*func_ptr_t1)(); +typedef void (*func_ptr_t2)(int x, char *y); +typedef s (*func_ptr_t3)(); + +func_ptr_t1 func_ptr1 = &func1; // COMPLIANT +func_ptr_t2 func_ptr2 = func2; // NON-COMPLIANT +func_ptr_t3 func_ptr3 = func3 + 0; // NON-COMPLIANT + +void take_func(func_ptr_t1 f1, func_ptr_t2 f2); + +func_ptr_t1 returns_func(int x) { + if (x == 0) { + return func1; // NON-COMPLIANT + } else if (x == 1) { + return &func1; // COMPLIANT + } + + return returns_func(0); // COMPLIANT +} + +#define MACRO_IDENTITY(f) (f) +#define MACRO_INVOKE_RISKY(f) (f()) +#define MACRO_INVOKE_IMPROVED(f) ((f)()) +#define MACRO_INVOKE_AND_USE_AS_TOKEN(f) f(0, #f) + +void test() { + func1(); // COMPLIANT + func2(1, "hello"); // COMPLIANT + + func1; // NON-COMPLIANT + func2; // NON-COMPLIANT + + &func1; // COMPLIANT + &func2; // COMPLIANT + + (func1)(); // COMPLIANT + (func2)(1, "hello"); // COMPLIANT + + &(func1); // COMPLIANT + &(func2); // COMPLIANT + + (&func1)(); // COMPLIANT + (&func2)(1, "hello"); // COMPLIANT + + (func1()); // COMPLIANT + (func2(1, "hello")); // COMPLIANT + + take_func(&func1, &func2); // COMPLIANT + take_func(func1, &func2); // NON-COMPLIANT + take_func(&func1, func2); // NON-COMPLIANT + take_func(func1, func2); // NON-COMPLIANT + + returns_func(0); // COMPLIANT + returns_func(0)(); // COMPLIANT + (returns_func(0))(); // COMPLIANT + + (void *)&func1; // COMPLIANT + (void *)(&func1); // COMPLIANT + (void *)func1; // NON-COMPLIANT + (void *)(func1); // NON-COMPLIANT + ((void *)func1); // NON-COMPLIANT + + MACRO_IDENTITY(func1); // NON-COMPLIANT + MACRO_IDENTITY(func1)(); // NON-COMPLIANT[FALSE NEGATIVE] + MACRO_IDENTITY(&func1); // COMPLIANT + MACRO_IDENTITY (&func1)(); // COMPLIANT + + MACRO_INVOKE_RISKY(func3); // NON-COMPLIANT[FALSE NEGATIVE] + MACRO_INVOKE_IMPROVED(func3); // NON-COMPLIANT[FALSE NEGATIVE] + MACRO_INVOKE_IMPROVED(&func3); // COMPLIANT + + MACRO_INVOKE_AND_USE_AS_TOKEN(func1); // COMPLIANT + + // Function pointers are exempt from this rule. + func_ptr1(); // COMPLIANT + func_ptr2(1, "hello"); // COMPLIANT + func_ptr1; // COMPLIANT + func_ptr2; // COMPLIANT + &func_ptr1; // COMPLIANT + &func_ptr2; // COMPLIANT + (func_ptr1)(); // COMPLIANT + (func_ptr2)(1, "hello"); // COMPLIANT + (*func_ptr1)(); // COMPLIANT + (*func_ptr2)(1, "hello"); // COMPLIANT + take_func(func_ptr1, func_ptr2); // COMPLIANT + (void *)func_ptr1; // COMPLIANT + (void *)&func_ptr1; // COMPLIANT + (void *)(&func_ptr1); // COMPLIANT + (void *)func_ptr1; // COMPLIANT + (void *)(func_ptr1); // COMPLIANT + ((void *)func_ptr1); // COMPLIANT + MACRO_IDENTITY(func_ptr1); // COMPLIANT + MACRO_IDENTITY(func_ptr1)(); // COMPLIANT + MACRO_IDENTITY(&func_ptr1); // COMPLIANT + (*MACRO_IDENTITY(&func_ptr1))(); // COMPLIANT + MACRO_INVOKE_RISKY(func_ptr3); // COMPLIANT + MACRO_INVOKE_IMPROVED(func_ptr3); // COMPLIANT + MACRO_INVOKE_IMPROVED(*&func_ptr3); // COMPLIANT +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/FunctionTypes.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/FunctionTypes.qll new file mode 100644 index 0000000000..3d6faadb42 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/FunctionTypes.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype FunctionTypesQuery = TFunctionAddressesShouldAddressOperatorQuery() + +predicate isFunctionTypesQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `functionAddressesShouldAddressOperator` query + FunctionTypesPackage::functionAddressesShouldAddressOperatorQuery() and + queryId = + // `@id` for the `functionAddressesShouldAddressOperator` query + "c/misra/function-addresses-should-address-operator" and + ruleId = "RULE-17-12" and + category = "advisory" +} + +module FunctionTypesPackage { + Query functionAddressesShouldAddressOperatorQuery() { + //autogenerate `Query` type + result = + // `Query` type for `functionAddressesShouldAddressOperator` query + TQueryC(TFunctionTypesPackageQuery(TFunctionAddressesShouldAddressOperatorQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index b10fbf0a2f..581585da5c 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -29,6 +29,7 @@ import Declarations8 import EssentialTypes import Expressions import FloatingTypes +import FunctionTypes import IO1 import IO2 import IO3 @@ -102,6 +103,7 @@ newtype TCQuery = TEssentialTypesPackageQuery(EssentialTypesQuery q) or TExpressionsPackageQuery(ExpressionsQuery q) or TFloatingTypesPackageQuery(FloatingTypesQuery q) or + TFunctionTypesPackageQuery(FunctionTypesQuery q) or TIO1PackageQuery(IO1Query q) or TIO2PackageQuery(IO2Query q) or TIO3PackageQuery(IO3Query q) or @@ -175,6 +177,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isEssentialTypesQueryMetadata(query, queryId, ruleId, category) or isExpressionsQueryMetadata(query, queryId, ruleId, category) or isFloatingTypesQueryMetadata(query, queryId, ruleId, category) or + isFunctionTypesQueryMetadata(query, queryId, ruleId, category) or isIO1QueryMetadata(query, queryId, ruleId, category) or isIO2QueryMetadata(query, queryId, ruleId, category) or isIO3QueryMetadata(query, queryId, ruleId, category) or diff --git a/docs/user_manual.md b/docs/user_manual.md index 5799d73e6f..db0f836339 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -78,7 +78,7 @@ The datasheet _"CodeQL Coding Standards: supported rules"_, provided with each r [^1]: AUTOSAR C++ versions R22-11, R21-11, R20-11, R19-11 and R19-03 are all identical as indicated in the document change history. [^2]: The unimplemented supportable AUTOSAR rules are `A7-1-8` and `A8-2-1`. These rules require additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules. -[^3]: The unimplemented supportable MISRA C 2012 rules are `Rule 9.5` and `Dir 4.14`. `Rule 9.5` requires additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules. `Dir 4.14` is covered by the default CodeQL queries, which identify potential security vulnerabilities caused by not validating external input. +[^3]: The unimplemented supportable MISRA C 2012 rules are `Rule 9.5`, `Rule 17.13`, and `Dir 4.14`. `Rule 9.5` and `Rule 17.13` require additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules. `Dir 4.14` is covered by the default CodeQL queries, which identify potential security vulnerabilities caused by not validating external input. [^4]: The rules 5.13.7, 19.0.1 and 19.1.2 are not planned to be implemented by CodeQL as they are compiler checked in all supported compilers. ## Supported environment diff --git a/rule_packages/c/FunctionTypes.json b/rule_packages/c/FunctionTypes.json new file mode 100644 index 0000000000..d9d8b6496d --- /dev/null +++ b/rule_packages/c/FunctionTypes.json @@ -0,0 +1,24 @@ +{ + "MISRA-C-2012": { + "RULE-17-12": { + "properties": { + "obligation": "advisory" + }, + "queries": [ + { + "description": "A function identifier should only be called with a parenthesized parameter list or used with a & (address-of).", + "kind": "problem", + "name": "A function identifier should only be called with a parenthesized parameter list or used with a &", + "precision": "very-high", + "severity": "error", + "short_name": "FunctionAddressesShouldAddressOperator", + "tags": [ + "readability", + "external/misra/c/2012/amendment3" + ] + } + ], + "title": "A function identifier should only be called with a parenthesized parameter list or used with a & (address-of)" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index b5e15ba6f6..475ea1d66c 100644 --- a/rules.csv +++ b/rules.csv @@ -739,7 +739,7 @@ c,MISRA-C-2012,RULE-17-9,Yes,Mandatory,,,Verify that a function declared with _N c,MISRA-C-2012,RULE-17-10,Yes,Required,,,A function declared with _noreturn shall have a return type of void,,NoReturn,Easy, c,MISRA-C-2012,RULE-17-11,Yes,Advisory,,,A function without a branch that returns shall be declared with _Noreturn,,NoReturn,Easy, c,MISRA-C-2012,RULE-17-12,Yes,Advisory,,,A function identifier should only be called with a parenthesized parameter list or used with a & (address-of),,FunctionTypes,Easy, -c,MISRA-C-2012,RULE-17-13,Yes,Required,,,"A function type shall not include any type qualifiers (const, volatile, restrict, or _Atomic)",,FunctionTypes,Easy, +c,MISRA-C-2012,RULE-17-13,No,Required,,,"A function type shall not include any type qualifiers (const, volatile, restrict, or _Atomic)",,,Easy, c,MISRA-C-2012,RULE-18-1,Yes,Required,,,A pointer resulting from arithmetic on a pointer operand shall address an element of the same array as that pointer operand,M5-0-16,Pointers1,Import, c,MISRA-C-2012,RULE-18-2,Yes,Required,,,Subtraction between pointers shall only be applied to pointers that address elements of the same array,M5-0-17,Pointers1,Import, c,MISRA-C-2012,RULE-18-3,Yes,Required,,,"The relational operators >, >=, < and <= shall not be applied to objects of pointer type except where they point into the same object",M5-0-18,Pointers1,Import,