diff --git a/c/common/src/codingstandards/c/UndefinedBehavior.qll b/c/common/src/codingstandards/c/UndefinedBehavior.qll index 5c9dc230d8..bbbb08678b 100644 --- a/c/common/src/codingstandards/c/UndefinedBehavior.qll +++ b/c/common/src/codingstandards/c/UndefinedBehavior.qll @@ -6,28 +6,53 @@ import codingstandards.cpp.UndefinedBehavior */ abstract class CUndefinedBehavior extends UndefinedBehavior { } +class PointerOrArrayType extends DerivedType { + PointerOrArrayType() { + this instanceof PointerType or + this instanceof ArrayType + } +} + +Type get(Function main) { + main.getName() = "main" and + main.getNumberOfParameters() = 2 and + main.getType().getUnderlyingType() instanceof IntType and + main.getParameter(0).getType().getUnderlyingType() instanceof IntType and + result = main.getParameter(1).getType().getUnderlyingType().(PointerOrArrayType).getBaseType() +} + +/** + * A function which has the signature - but not the name - of a main function. + */ class C99MainFunction extends Function { C99MainFunction() { this.getNumberOfParameters() = 2 and - this.getType() instanceof IntType and - this.getParameter(0).getType() instanceof IntType and - this.getParameter(1).getType().(PointerType).getBaseType().(PointerType).getBaseType() - instanceof CharType + this.getType().getUnderlyingType() instanceof IntType and + this.getParameter(0).getType().getUnderlyingType() instanceof IntType and + this.getParameter(1) + .getType() + .getUnderlyingType() + .(PointerOrArrayType) + .getBaseType() + .(PointerOrArrayType) + .getBaseType() instanceof CharType or this.getNumberOfParameters() = 0 and - this.getType() instanceof VoidType + // Must be explicitly declared as `int main(void)`. + this.getADeclarationEntry().hasVoidParamList() and + this.getType().getUnderlyingType() instanceof IntType } } class CUndefinedMainDefinition extends CUndefinedBehavior, Function { CUndefinedMainDefinition() { // for testing purposes, we use the prefix ____codeql_coding_standards` - (this.getName() = "main" or this.getName().indexOf("____codeql_coding_standards") = 0) and + (this.getName() = "main" or this.getName().indexOf("____codeql_coding_standards_main") = 0) and not this instanceof C99MainFunction } override string getReason() { result = - "The behavior of the program is undefined because the main function is not defined according to the C standard." + "main function may trigger undefined behavior because it is not in one of the formats specified by the C standard." } } diff --git a/c/misra/src/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.ql b/c/misra/src/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.ql index 53f72e6bee..00ef875985 100644 --- a/c/misra/src/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.ql +++ b/c/misra/src/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.ql @@ -18,4 +18,4 @@ import codingstandards.c.UndefinedBehavior from CUndefinedBehavior c where not isExcluded(c, Language3Package::occurrenceOfUndefinedBehaviorQuery()) -select c, "May result in undefined behavior." +select c, c.getReason() diff --git a/c/misra/test/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.expected b/c/misra/test/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.expected index 68216d500f..1e57f92e4a 100644 --- a/c/misra/test/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.expected +++ b/c/misra/test/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.expected @@ -1,5 +1,8 @@ -| test.c:8:6:8:35 | ____codeql_coding_standards_m2 | May result in undefined behavior. | -| test.c:11:5:11:34 | ____codeql_coding_standards_m3 | May result in undefined behavior. | -| test.c:15:5:15:34 | ____codeql_coding_standards_m4 | May result in undefined behavior. | -| test.c:19:5:19:34 | ____codeql_coding_standards_m5 | May result in undefined behavior. | -| test.c:23:5:23:34 | ____codeql_coding_standards_m6 | May result in undefined behavior. | +| test.c:4:6:4:38 | ____codeql_coding_standards_main1 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:8:5:8:37 | ____codeql_coding_standards_main2 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:27:5:27:37 | ____codeql_coding_standards_main6 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:32:6:32:38 | ____codeql_coding_standards_main7 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:36:5:36:37 | ____codeql_coding_standards_main8 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:40:5:40:37 | ____codeql_coding_standards_main9 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:44:5:44:38 | ____codeql_coding_standards_main10 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | +| test.c:48:5:48:38 | ____codeql_coding_standards_main11 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. | diff --git a/c/misra/test/rules/RULE-1-3/test.c b/c/misra/test/rules/RULE-1-3/test.c index 190cff4000..fd54959f56 100644 --- a/c/misra/test/rules/RULE-1-3/test.c +++ b/c/misra/test/rules/RULE-1-3/test.c @@ -1,25 +1,50 @@ -void main(void) { // COMPLIANT +int main(void) { // COMPLIANT } -int ____codeql_coding_standards_m1(int argc, char **argv) { // NON_COMPLIANT +void ____codeql_coding_standards_main1(void) { // NON_COMPLIANT return 0; } -void ____codeql_coding_standards_m2(char *argc, char **argv) { // NON_COMPLIANT +int ____codeql_coding_standards_main2() { // NON_COMPLIANT + return 0; +} + +int ____codeql_coding_standards_main3(int argc, char **argv) { // COMPLIANT + return 0; +} + +int ____codeql_coding_standards_main4(int argc, char argv[][]) { // COMPLIANT + return 0; +} + +int ____codeql_coding_standards_main5(int argc, char *argv[]) { // COMPLIANT + return 0; +} + +typedef int MY_INT; +typedef char *MY_CHAR_PTR; + +int ____codeql_coding_standards_main6(MY_INT argc, + MY_CHAR_PTR argv[]) { // COMPLIANT + return 0; +} + +void ____codeql_coding_standards_main7(char *argc, + char **argv) { // NON_COMPLIANT } -int ____codeql_coding_standards_m3(int argc, char *argv) { // NON_COMPLIANT +int ____codeql_coding_standards_main8(int argc, char *argv) { // NON_COMPLIANT return 0; } -int ____codeql_coding_standards_m4() { // NON_COMPLIANT +int ____codeql_coding_standards_main9() { // NON_COMPLIANT return 0; } -int ____codeql_coding_standards_m5(int argc, int *argv) { // NON_COMPLIANT +int ____codeql_coding_standards_main10(int argc, int *argv) { // NON_COMPLIANT return 0; } -int ____codeql_coding_standards_m6(int argc, int **argv) { // NON_COMPLIANT +int ____codeql_coding_standards_main11(int argc, int **argv) { // NON_COMPLIANT return 0; } diff --git a/change_notes/2024-10-21-rule-1-3-main.md b/change_notes/2024-10-21-rule-1-3-main.md new file mode 100644 index 0000000000..7bd8d4bd54 --- /dev/null +++ b/change_notes/2024-10-21-rule-1-3-main.md @@ -0,0 +1,3 @@ + - `RULE-1-3` - `OccurrenceOfUndefinedBehavior.ql`: + - Improve alert message to report the undefined behavior triggered. + - Address both false positives and false negatives in identifying standard compliant main methods. Previously, `void main()` was considered permitted and `int main(void)` banned. In addition, we now detect main methods as standard compliant if they use typedefs, and if arrays are used in the definition of `argv`. \ No newline at end of file