From c9218647fb1f13e3cf1793c8f6acca5a55fcb1f9 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Wed, 13 Jul 2022 12:17:21 +0200 Subject: [PATCH] Release v2.4.0 --- .github/workflows/code-scanning-pack-gen.yml | 2 +- .vscode/tasks.json | 12 +- c/cert/src/qlpack.yml | 2 +- ...formFileOperationsOnDevices-standard.qhelp | 594 ++++++++++++++++++ .../DoNotPerformFileOperationsOnDevices.qhelp | 18 + .../DoNotPerformFileOperationsOnDevices.ql | 64 ++ ...getwsMayReturnAnEmptyString-standard.qhelp | 392 ++++++++++++ ...lFgetsOrFgetwsMayReturnAnEmptyString.qhelp | 18 + ...sfulFgetsOrFgetwsMayReturnAnEmptyString.ql | 48 ++ .../ResetStringsOnFgetsOrFgetwsFailure.ql | 3 - ...nctionsRepresentableAsUChar-standard.qhelp | 33 + ...andlingFunctionsRepresentableAsUChar.qhelp | 18 + ...erHandlingFunctionsRepresentableAsUChar.ql | 29 + .../{FIO39-C => STR37-C}/standard-example.c | 0 c/cert/test/qlpack.yml | 2 +- ...NotPerformFileOperationsOnDevices.expected | 40 ++ .../DoNotPerformFileOperationsOnDevices.qlref | 1 + c/cert/test/rules/FIO32-C/test.c | 50 ++ .../EndOfFileCheckPortability.expected | 1 - c/cert/test/rules/FIO34-C/test.c | 2 +- ...etsOrFgetwsMayReturnAnEmptyString.expected | 1 + ...lFgetsOrFgetwsMayReturnAnEmptyString.qlref | 1 + c/cert/test/rules/FIO37-C/test.c | 56 ++ ...lingFunctionsRepresentableAsUChar.expected | 28 + ...andlingFunctionsRepresentableAsUChar.qlref | 1 + c/cert/test/rules/STR37-C/test.c | 86 +++ c/common/src/qlpack.yml | 2 +- .../test/{ => library}/expr/FullExpr.expected | 0 c/common/test/{ => library}/expr/FullExpr.ql | 0 c/common/test/{ => library}/expr/fullexpr.c | 0 .../fgetserrormanagement/FgetsGuard.expected | 20 + .../fgetserrormanagement/FgetsGuard.ql | 5 + .../test/library/fgetserrormanagement/test.c | 195 ++++++ c/common/test/qlpack.yml | 2 +- .../HashOperatorsUsed.expected | 4 - .../hashoperatorsused/HashOperatorsUsed.ql | 2 - c/common/test/rules/hashoperatorsused/test.c | 19 - .../IncludeGuardsNotUsed.expected | 4 + .../IncludeGuardsNotUsed.ql | 2 + .../includeguardsnotused/headers/test1.h | 0 .../includeguardsnotused/headers/test2.h | 0 .../includeguardsnotused/headers/test3.h | 0 .../includeguardsnotused/headers/test4.h | 0 .../includeguardsnotused/headers/test5.h | 0 .../includeguardsnotused/headers/test6.h | 5 + .../includeguardsnotused/headers/test7.h | 5 + .../test/rules/includeguardsnotused/test.c | 14 + c/misra/src/qlpack.yml | 2 +- ...oreThanOneHashOperatorInMacroDefinition.ql | 26 + .../src/rules/RULE-20-11/standard-example.c | 3 + .../MacroParameterUsedAsHashOperand.ql | 39 ++ .../src/rules/RULE-20-12/standard-example.c | 7 + .../rules/RULE-20-5/UndefShouldNotBeUsed.ql | 19 + .../src/rules/RULE-20-5/standard-example.c | 7 + ...leOpenForReadAndWriteOnDifferentStreams.ql | 68 ++ .../src/rules/RULE-22-3/standard-example.c | 5 + .../AttemptToWriteToAReadOnlyStream.ql | 36 ++ .../src/rules/RULE-22-4/standard-example.c | 6 + .../PointerToAFileObjectDereferenced.ql | 38 ++ .../src/rules/RULE-22-5/standard-example.c | 8 + ...allBeComparedWithUnmodifiedReturnValues.ql | 60 ++ .../src/rules/RULE-22-7/standard-example1.c | 10 + .../src/rules/RULE-22-7/standard-example2.c | 19 + .../PrecautionIncludeGuardsNotProvided.ql | 24 + .../src/rules/RULE-4-10/standard-example.c | 12 + c/misra/test/qlpack.yml | 2 +- ...nOneHashOperatorInMacroDefinition.expected | 1 + ...ThanOneHashOperatorInMacroDefinition.qlref | 1 + c/misra/test/rules/RULE-20-11/test.c | 27 + .../MacroParameterUsedAsHashOperand.expected | 2 + .../MacroParameterUsedAsHashOperand.qlref | 1 + c/misra/test/rules/RULE-20-12/test.c | 25 + .../RULE-20-5/UndefShouldNotBeUsed.expected | 1 + .../RULE-20-5/UndefShouldNotBeUsed.qlref | 1 + c/misra/test/rules/RULE-20-5/test.c | 2 + ...ForReadAndWriteOnDifferentStreams.expected | 5 + ...penForReadAndWriteOnDifferentStreams.qlref | 1 + c/misra/test/rules/RULE-22-3/test.c | 60 ++ .../AttemptToWriteToAReadOnlyStream.expected | 2 + .../AttemptToWriteToAReadOnlyStream.qlref | 1 + c/misra/test/rules/RULE-22-4/test.c | 21 + .../PointerToAFileObjectDereferenced.expected | 7 + .../PointerToAFileObjectDereferenced.qlref | 1 + c/misra/test/rules/RULE-22-5/test.c | 28 + ...omparedWithUnmodifiedReturnValues.expected | 2 + ...BeComparedWithUnmodifiedReturnValues.qlref | 1 + c/misra/test/rules/RULE-22-7/test.c | 31 + .../RULE-4-10/NonUniqueIncludeGuards.testref | 1 + ...PrecautionIncludeGuardsNotProvided.testref | 1 + ...-20-M16-3-1-exclude-non-function-macros.md | 2 + cpp/autosar/src/qlpack.yml | 2 +- .../rules/M16-2-3/IncludeGuardsNotProvided.ql | 52 +- ...OccurrenceHashOperatorInMacroDefinition.ql | 19 +- cpp/autosar/test/qlpack.yml | 2 +- .../M16-2-3/IncludeGuardsNotProvided.expected | 3 - .../M16-2-3/IncludeGuardsNotProvided.qlref | 1 - .../M16-2-3/IncludeGuardsNotProvided.testref | 1 + .../M16-2-3/NonUniqueIncludeGuardsCpp.testref | 1 + ...enceHashOperatorInMacroDefinition.expected | 1 - cpp/autosar/test/rules/M16-3-1/test.cpp | 3 +- cpp/cert/src/qlpack.yml | 2 +- cpp/cert/test/qlpack.yml | 2 +- .../src/codingstandards/cpp/CharFunctions.qll | 31 + .../src/codingstandards/cpp/Dereferenced.qll | 3 + .../cpp/FgetsErrorManagement.qll | 93 +-- cpp/common/src/codingstandards/cpp/Macro.qll | 65 ++ .../codingstandards/cpp/ReadErrorsAndEOF.qll | 6 +- .../codingstandards/cpp/exclusions/c/IO3.qll | 106 ++++ .../cpp/exclusions/c/Preprocessor2.qll | 74 +++ .../cpp/exclusions/c/RuleMetadata.qll | 9 + .../cpp/exclusions/c/Strings2.qll | 25 + .../IncludeGuardsNotUsed.qll | 40 ++ .../cpp/standardlibrary/FileAccess.qll | 21 + cpp/common/src/qlpack.yml | 2 +- cpp/common/test/qlpack.yml | 2 +- .../IncludeGuardsNotUsed.expected | 4 + .../IncludeGuardsNotUsed.ql | 2 + .../includeguardsnotused/headers/test1.hpp | 4 + .../includeguardsnotused/headers/test2.hpp | 4 + .../includeguardsnotused/headers/test3.hpp | 2 + .../includeguardsnotused/headers/test4.hpp | 4 + .../includeguardsnotused/headers/test5.hpp | 6 + .../includeguardsnotused/headers/test6.hpp | 5 + .../test/rules/includeguardsnotused}/test.cpp | 4 +- cpp/misra/src/qlpack.yml | 2 +- cpp/misra/test/qlpack.yml | 2 +- cpp/report/src/qlpack.yml | 2 +- rule_packages/c/IO3.json | 139 ++++ rule_packages/c/Preprocessor2.json | 93 +++ rule_packages/c/Strings2.json | 24 + rule_packages/cpp/Includes.json | 7 +- rule_packages/cpp/Macros.json | 6 +- rules.csv | 34 +- .../generate_rules/generate_package_files.py | 89 +-- .../templates/exclusions.qll.template | 4 + 135 files changed, 3186 insertions(+), 214 deletions(-) create mode 100644 c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices-standard.qhelp create mode 100644 c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qhelp create mode 100644 c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql create mode 100644 c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString-standard.qhelp create mode 100644 c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qhelp create mode 100644 c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql create mode 100644 c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar-standard.qhelp create mode 100644 c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qhelp create mode 100644 c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql rename c/cert/src/rules/{FIO39-C => STR37-C}/standard-example.c (100%) create mode 100644 c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected create mode 100644 c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qlref create mode 100644 c/cert/test/rules/FIO32-C/test.c create mode 100644 c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.expected create mode 100644 c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qlref create mode 100644 c/cert/test/rules/FIO37-C/test.c create mode 100644 c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected create mode 100644 c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qlref create mode 100644 c/cert/test/rules/STR37-C/test.c rename c/common/test/{ => library}/expr/FullExpr.expected (100%) rename c/common/test/{ => library}/expr/FullExpr.ql (100%) rename c/common/test/{ => library}/expr/fullexpr.c (100%) create mode 100644 c/common/test/library/fgetserrormanagement/FgetsGuard.expected create mode 100644 c/common/test/library/fgetserrormanagement/FgetsGuard.ql create mode 100644 c/common/test/library/fgetserrormanagement/test.c delete mode 100644 c/common/test/rules/hashoperatorsused/HashOperatorsUsed.expected delete mode 100644 c/common/test/rules/hashoperatorsused/HashOperatorsUsed.ql delete mode 100644 c/common/test/rules/hashoperatorsused/test.c create mode 100644 c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.expected create mode 100644 c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql rename cpp/autosar/test/rules/M16-2-3/headers/test1.hpp => c/common/test/rules/includeguardsnotused/headers/test1.h (100%) rename cpp/autosar/test/rules/M16-2-3/headers/test2.hpp => c/common/test/rules/includeguardsnotused/headers/test2.h (100%) rename cpp/autosar/test/rules/M16-2-3/headers/test3.hpp => c/common/test/rules/includeguardsnotused/headers/test3.h (100%) rename cpp/autosar/test/rules/M16-2-3/headers/test4.hpp => c/common/test/rules/includeguardsnotused/headers/test4.h (100%) rename cpp/autosar/test/rules/M16-2-3/headers/test5.hpp => c/common/test/rules/includeguardsnotused/headers/test5.h (100%) create mode 100644 c/common/test/rules/includeguardsnotused/headers/test6.h create mode 100644 c/common/test/rules/includeguardsnotused/headers/test7.h create mode 100644 c/common/test/rules/includeguardsnotused/test.c create mode 100644 c/misra/src/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.ql create mode 100644 c/misra/src/rules/RULE-20-11/standard-example.c create mode 100644 c/misra/src/rules/RULE-20-12/MacroParameterUsedAsHashOperand.ql create mode 100644 c/misra/src/rules/RULE-20-12/standard-example.c create mode 100644 c/misra/src/rules/RULE-20-5/UndefShouldNotBeUsed.ql create mode 100644 c/misra/src/rules/RULE-20-5/standard-example.c create mode 100644 c/misra/src/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.ql create mode 100644 c/misra/src/rules/RULE-22-3/standard-example.c create mode 100644 c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql create mode 100644 c/misra/src/rules/RULE-22-4/standard-example.c create mode 100644 c/misra/src/rules/RULE-22-5/PointerToAFileObjectDereferenced.ql create mode 100644 c/misra/src/rules/RULE-22-5/standard-example.c create mode 100644 c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql create mode 100644 c/misra/src/rules/RULE-22-7/standard-example1.c create mode 100644 c/misra/src/rules/RULE-22-7/standard-example2.c create mode 100644 c/misra/src/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.ql create mode 100644 c/misra/src/rules/RULE-4-10/standard-example.c create mode 100644 c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.expected create mode 100644 c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.qlref create mode 100644 c/misra/test/rules/RULE-20-11/test.c create mode 100644 c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.expected create mode 100644 c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.qlref create mode 100644 c/misra/test/rules/RULE-20-12/test.c create mode 100644 c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.expected create mode 100644 c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.qlref create mode 100644 c/misra/test/rules/RULE-20-5/test.c create mode 100644 c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.expected create mode 100644 c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.qlref create mode 100644 c/misra/test/rules/RULE-22-3/test.c create mode 100644 c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.expected create mode 100644 c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.qlref create mode 100644 c/misra/test/rules/RULE-22-4/test.c create mode 100644 c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.expected create mode 100644 c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.qlref create mode 100644 c/misra/test/rules/RULE-22-5/test.c create mode 100644 c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.expected create mode 100644 c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.qlref create mode 100644 c/misra/test/rules/RULE-22-7/test.c create mode 100644 c/misra/test/rules/RULE-4-10/NonUniqueIncludeGuards.testref create mode 100644 c/misra/test/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.testref create mode 100644 change_notes/2022-05-20-M16-3-1-exclude-non-function-macros.md delete mode 100644 cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.expected delete mode 100644 cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.qlref create mode 100644 cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.testref create mode 100644 cpp/autosar/test/rules/M16-2-3/NonUniqueIncludeGuardsCpp.testref create mode 100644 cpp/common/src/codingstandards/cpp/CharFunctions.qll create mode 100644 cpp/common/src/codingstandards/cpp/Macro.qll create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/IO3.qll create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Preprocessor2.qll create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Strings2.qll create mode 100644 cpp/common/src/codingstandards/cpp/rules/includeguardsnotused/IncludeGuardsNotUsed.qll create mode 100644 cpp/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.expected create mode 100644 cpp/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test1.hpp create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test2.hpp create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test3.hpp create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test4.hpp create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test5.hpp create mode 100644 cpp/common/test/rules/includeguardsnotused/headers/test6.hpp rename cpp/{autosar/test/rules/M16-2-3 => common/test/rules/includeguardsnotused}/test.cpp (57%) create mode 100644 rule_packages/c/IO3.json create mode 100644 rule_packages/c/Preprocessor2.json create mode 100644 rule_packages/c/Strings2.json diff --git a/.github/workflows/code-scanning-pack-gen.yml b/.github/workflows/code-scanning-pack-gen.yml index fdad8f932..b734cdd5e 100644 --- a/.github/workflows/code-scanning-pack-gen.yml +++ b/.github/workflows/code-scanning-pack-gen.yml @@ -73,7 +73,7 @@ jobs: PATH=$PATH:$CODEQL_HOME/codeql pip install -r scripts/requirements.txt find rule_packages/cpp -name '*.json' -exec basename {} .json \; | xargs --max-procs "$XARGS_MAX_PROCS" --max-args 1 python3 scripts/generate_rules/generate_package_files.py -a cpp - find rule_packages/c -name '*.json' -exec basename {} .json \; | xargs --max-procs "$XARGS_MAX_PROCS" --max-args 1 python3 scripts/generate_rules/generate_package_files.py -a c + find rule_packages/c -name '*.json' -exec basename {} .json \; | xargs --max-procs "$XARGS_MAX_PROCS" --max-args 1 python3 scripts/generate_rules/generate_package_files.py --skip-shared-test-generation -a c echo "Generating help markdown file for cert" $CODEQL_LATEST_HOME/codeql/codeql generate query-help -vvv --format=markdown -o cpp/cert/src/ cpp/cert/src/rules diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 24432cd31..8e5b1b4cc 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -180,7 +180,9 @@ "Expressions", "Freed", "Functions", - "IO", + "IO1", + "IO2", + "IO3", "Includes", "Initialization", "IntegerConversion", @@ -205,9 +207,17 @@ "Strings", "Strings1", "Strings2", + "Strings3", "Syntax", "Templates", "TypeRanges", + "Lambdas", + "Pointers", + "Preprocessor1", + "Preprocessor2", + "IntegerConversion", + "Expressions", + "DeadCode" "VirtualFunctions" ] }, diff --git a/c/cert/src/qlpack.yml b/c/cert/src/qlpack.yml index 9ecaaadc7..b3cd47c3a 100644 --- a/c/cert/src/qlpack.yml +++ b/c/cert/src/qlpack.yml @@ -1,4 +1,4 @@ name: cert-c-coding-standards -version: 2.3.0 +version: 2.4.0 suites: codeql-suites libraryPathDependencies: common-c-coding-standards \ No newline at end of file diff --git a/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices-standard.qhelp b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices-standard.qhelp new file mode 100644 index 000000000..44fd54fff --- /dev/null +++ b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices-standard.qhelp @@ -0,0 +1,594 @@ + + +
+

File names on many operating systems, including Windows and UNIX, may be used to access special files, which are actually devices. Reserved Microsoft Windows device names include AUX, CON, PRN, COM1, and LPT1 or paths using the \\.\ device namespace. Device files on UNIX systems are used to apply access rights and to direct operations on the files to the appropriate device drivers.

+

Performing operations on device files that are intended for ordinary character or binary files can result in crashes and denial-of-service attacks. For example, when Windows attempts to interpret the device name as a file resource, it performs an invalid resource access that usually results in a crash [Howard 2002].

+

Device files in UNIX can be a security risk when an attacker can access them in an unauthorized way. For example, if attackers can read or write to the /dev/kmem device, they may be able to alter the priority, UID, or other attributes of their process or simply crash the system. Similarly, access to disk devices, tape devices, network devices, and terminals being used by other processes can lead to problems [Garfinkel 1996].

+

On Linux, it is possible to lock certain applications by attempting to open devices rather than files. Consider the following example:

+ /dev/mouse +/dev/console +/dev/tty0 +/dev/zero + +

A Web browser that failed to check for these devices would allow an attacker to create a website with image tags such as <IMG src="file:///dev/mouse"> that would lock the user's mouse [Howard 2002].

+
+
+

In this noncompliant code example, the user can specify a locked device or a FIFO (first-in, first-out) file name, which can cause the program to hang on the call to fopen():

+ #include <stdio.h> +  +void func(const char *file_name) { + FILE *file; + if ((file = fopen(file_name, "wb")) == NULL) { + /* Handle error */ + } + + /* Operate on the file */ + + if (fclose(file) == EOF) { + /* Handle error */ + } +} +
+
+

POSIX defines the O_NONBLOCK flag to open(), which ensures that delayed operations on a file do not hang the program [IEEE Std 1003.1:2013].

+
+

When opening a FIFO with O_RDONLY or O_WRONLY set:

+
    +
  • If O_NONBLOCK is set, an open() for reading-only returns without delay. An open() for writing-only returns an error if no process currently has the file open for reading.
    • +
  • If O_NONBLOCK is clear, an open() for reading-only blocks the calling thread until a thread opens the file for writing. An open() for writing-only blocks the calling thread until a thread opens the file for reading.
    • +
+

When opening a block special or character special file that supports nonblocking opens:

+
    +
  • If O_NONBLOCK is set, the open() function returns without blocking for the device to be ready or available; subsequent behavior is device-specific.
    • +
  • If O_NONBLOCK is clear, the open() function blocks the calling thread until the device is ready or available before returning.
    • +
+

Otherwise, the behavior of O_NONBLOCK is unspecified.

+
+

Once the file is open, programmers can use the POSIX lstat() and fstat() functions to obtain information about a file and the S_ISREG() macro to determine if the file is a regular file. 

+

Because the behavior of O_NONBLOCK on subsequent calls to read() or write() is unspecified, it is advisable to disable the flag after it has been determined that the file in question is not a special device.

+

When available (Linux 2.1.126+, FreeBSD, Solaris 10, POSIX.1-2008), the O_NOFOLLOW flag should also be used. (See POS01-C. Check for the existence of links when dealing with files.) When O_NOFOLLOW is not available, symbolic link checks should use the method from POS35-C. Avoid race conditions while checking for the existence of a symbolic link.

+ #include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> + +#ifdef O_NOFOLLOW + #define OPEN_FLAGS O_NOFOLLOW | O_NONBLOCK +#else + #define OPEN_FLAGS O_NONBLOCK +#endif + +void func(const char *file_name) { + struct stat orig_st; + struct stat open_st; + int fd; + int flags; + + if ((lstat(file_name, &orig_st) != 0) || + (!S_ISREG(orig_st.st_mode))) { + /* Handle error */ + } + + /* Race window */ + + fd = open(file_name, OPEN_FLAGS | O_WRONLY); + if (fd == -1) { + /* Handle error */ + } + + if (fstat(fd, &open_st) != 0) { + /* Handle error */ + } + + if ((orig_st.st_mode != open_st.st_mode) || + (orig_st.st_ino != open_st.st_ino) || + (orig_st.st_dev != open_st.st_dev)) { + /* The file was tampered with */ + } + + /* + * Optional: drop the O_NONBLOCK now that we are sure + * this is a good file. +  */ + if ((flags = fcntl(fd, F_GETFL)) == -1) { + /* Handle error */ + } + + if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) == -1) { + /* Handle error */ + } + + /* Operate on the file */ + + if (close(fd) == -1) { + /* Handle error */ + } +} +

This code contains an intractable TOCTOU (time-of-check, time-of-use) race condition under which an attacker can alter the file referenced by file_name following the call to lstat() but before the call to open(). The switch will be discovered after the file is opened, but opening the file cannot be prevented in the case where this action itself causes undesired behavior. (See FIO45-C. Avoid TOCTOU race conditions while accessing files for more information about TOCTOU race conditions.)

+

Essentially, an attacker can switch out a file for one of the file types shown in the following table with the specified effect.

+

File Types and Effects

+ + + + + + + + + + + + + + + + + + + + + + + +
+ Type + + Note on Effect +
+ Another regular file + + The + fstat() + verification fails. +
+ FIFO + + Either + open() + returns + -1 + and sets + errno + to + ENXIO + , or + open() + succeeds and the + fstat() + verification fails. +
+ Symbolic link + + open() + returns + -1 + if + O_NOFOLLOW + is available; otherwise, the + fstat() + verification fails. +
+ Special device + + Usually the + fstat() + verification fails on + st_mode + . This can still be a problem if the device is one for which just opening (or closing) it causes a side effect. If + st_mode + compares equal, then the device is one that, after opening, appears to be a regular file. It would then fail the + fstat() + verification on + st_dev + and + st_ino + (unless it happens to be the + + same + + file, as can happen with + /dev/fd/* + on Solaris, but this would not be a problem). +
+

To be compliant with this rule and to prevent this TOCTOU race condition, file_name must refer to a file in a secure directory. (See FIO15-C. Ensure that file operations are performed in a secure directory.)

+
+
+

This noncompliant code example uses the GetFileType() function to attempt to prevent opening a special file: 

+ #include <Windows.h> + +void func(const TCHAR *file_name) { + HANDLE hFile = CreateFile( + file_name, + GENERIC_READ | GENERIC_WRITE, 0, + NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, NULL + ); + if (hFile == INVALID_HANDLE_VALUE) { + /* Handle error */ + } else if (GetFileType(hFile) != FILE_TYPE_DISK) { + /* Handle error */ + CloseHandle(hFile); + } else { + /* Operate on the file */ + CloseHandle(hFile); + } +} +

Although tempting, the Win32 GetFileType() function is dangerous in this case. If the file name given identifies a named pipe that is currently blocking on a read request, the call to GetFileType() will block until the read request completes. This provides an effective attack vector for a denial-of-service attack on the application. Furthermore, the act of opening a file handle may cause side effects, such as line states being set to their default voltage when opening a serial device.

+
+
+

Microsoft documents a list of reserved identifiers that represent devices and have a device namespace to be used specifically by devices [MSDN]. In this compliant solution, the isReservedName() function can be used to determine if a specified path refers to a device. Care must be taken to avoid a TOCTOU race condition when first testing a path name using the isReservedName() function and then later operating on that path name.

+ #include <ctype.h> +#include <stdbool.h> +#include <stdlib.h> +#include <string.h> +#include <stdio.h> + +static bool isReservedName(const char *path) { + /* This list of reserved names comes from MSDN */ + static const char *reserved[] = { + "nul", "con", "prn", "aux", "com1", "com2", "com3", + "com4", "com5", "com6", "com7", "com8", "com9", + "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", + "lpt7", "lpt8", "lpt9" + }; + bool ret = false; + +/* + * First, check to see if this is a device namespace, which + * always starts with \\.\, because device namespaces are not + * valid file paths. + */ + +  if (!path || 0 == strncmp(path, "\\\\.\\", 4)) { + return true; + } + + /* Compare against the list of ancient reserved names */ + for (size_t i = 0; !ret && + i < sizeof(reserved) / sizeof(*reserved); ++i) { + /* + * Because Windows uses a case-insensitive file system, operate on + * a lowercase version of the given filename. Note: This ignores + * globalization issues and assumes ASCII characters. + */ + if (0 == _stricmp(path, reserved[i])) { + ret = true; + } + } + return ret; +} +
+
+

Allowing operations that are appropriate only for regular files to be performed on devices can result in denial-of-service attacks or more serious exploits depending on the platform.

+ + + + + + + + + + + + + + + + + + + +
+ Rule + + Severity + + Likelihood + + Remediation Cost + + Priority + + Level +
+ FIO32-C + + Medium + + Unlikely + + Medium + + P4 + + L3 +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ Tool + + Version + + Checker + + Description +
+ + Compass/ROSE + + + + + Could detect some violations of this rule. This rule applies only to untrusted file name strings, and ROSE cannot tell which strings are trusted and which are not. The best heuristic is to note if there is any verification of the file name before or after the + fopen() + call. If there is any verification, then the file opening should be preceded by an + lstat() + call and succeeded by an + fstat() + call. Although that does not enforce the rule completely, it does indicate that the coder is aware of the + lstat-fopen + - + fstat + idiom +
+ + Helix QAC + + + 2022.1 + + C4921, C4922, C4923 + C++4921, C++4922, C++4923 + +
+ + Parasoft C/C++test + + + 2021.2 + + CERT_C-FIO32-a + + Protect against file name injection +
+ + Polyspace Bug Finder + + + R2022a + + + CERT C: Rule FIO32-C + + + Checks for inappropriate I/O operation on device files (rule fully covered) +
+ + PRQA QA-C + + + 9.7 + + 4921, 4922, 4923 + + Enforced by QAC +
+ + PRQA QA-C++ + + + 4.4 + + 4921, 4922, 4923 + +
+
+
+

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

+
+
+

Key here (explains table format and definitions)

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ Taxonomy + + Taxonomy item + + Relationship +
+ + CERT C Secure Coding Standard + + + + FIO05-C. Identify files using multiple file attributes + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
+ + CERT C Secure Coding Standard + + + + FIO15-C. Ensure that file operations are performed in a secure directory + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
+ + CERT C Secure Coding Standard + + + + POS01-C. Check for the existence of links when dealing with files + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
+ + CERT C Secure Coding Standard + + + + POS35-C. Avoid race conditions while checking for the existence of a symbolic link + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
+ + CERT Oracle Secure Coding Standard for Java + + + + FIO00-J. Do not operate on files in shared directories + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
+
+
+

Key here for mapping notes

+

CWE-67 and FIO32-C

+

FIO32-C = Union( CWE-67, list) where list =

+
    +
  • Treating trusted device names like regular files in Windows.
    • +
+
    +
  • Treating device names (both trusted and untrusted) like regular files in POSIX
    • +
+
+
+ + + + + + + + + + + + + + + + + + + +
+ [ + + Garfinkel 1996 + + ] + + Section 5.6, "Device Files" +
+ [ + + Howard 2002 + + ] + + Chapter 11, "Canonical Representation Issues" +
+ [ + + IEEE Std 1003.1:2013 + + ] + + XSH, System Interfaces, + open +
+ [ + + MSDN + + ] + +
+
+ \ No newline at end of file diff --git a/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qhelp b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qhelp new file mode 100644 index 000000000..439daed07 --- /dev/null +++ b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qhelp @@ -0,0 +1,18 @@ + + + + +

This query implements the CERT-C rule FIO32-C:

+
+

Do not perform operations on devices that are only appropriate for files

+
+ + + +
  • + CERT-C: + FIO32-C: Do not perform operations on devices that are only appropriate for files + . +
    • +     +     \ No newline at end of file diff --git a/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql new file mode 100644 index 000000000..2d16b2ffe --- /dev/null +++ b/c/cert/src/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql @@ -0,0 +1,64 @@ +/** + * @id c/cert/do-not-perform-file-operations-on-devices + * @name FIO32-C: Do not perform operations on devices that are only appropriate for files + * @description Performing file operations on devices can result in crashes. + * @kind path-problem + * @precision medium + * @problem.severity error + * @tags external/cert/id/fio32-c + * correctness + * security + * external/cert/obligation/rule + */ + +import cpp +import codingstandards.c.cert +import semmle.code.cpp.security.FunctionWithWrappers +import semmle.code.cpp.security.Security +import semmle.code.cpp.security.TaintTracking +import TaintedWithPath + +// Query TaintedPath.ql from the CodeQL standard library +/** + * A function for opening a file. + */ +class FileFunction extends FunctionWithWrappers { + FileFunction() { + exists(string nme | this.hasGlobalName(nme) | + nme = ["fopen", "_fopen", "_wfopen", "open", "_open", "_wopen"] + or + // create file function on windows + nme.matches("CreateFile%") + ) + or + this.hasQualifiedName("std", "fopen") + or + // on any of the fstream classes, or filebuf + exists(string nme | this.getDeclaringType().hasQualifiedName("std", nme) | + nme = ["basic_fstream", "basic_ifstream", "basic_ofstream", "basic_filebuf"] + ) and + // we look for either the open method or the constructor + (this.getName() = "open" or this instanceof Constructor) + } + + // conveniently, all of these functions take the path as the first parameter! + override predicate interestingArg(int arg) { arg = 0 } +} + +class TaintedPathConfiguration extends TaintTrackingConfiguration { + override predicate isSink(Element tainted) { + exists(FileFunction fileFunction | fileFunction.outermostWrapperFunctionCall(tainted, _)) + } +} + +from + FileFunction fileFunction, Expr taintedArg, Expr taintSource, PathNode sourceNode, + PathNode sinkNode, string taintCause, string callChain +where + not isExcluded(taintedArg, IO3Package::doNotPerformFileOperationsOnDevicesQuery()) and + fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and + taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and + isUserInput(taintSource, taintCause) +select taintedArg, sourceNode, sinkNode, + "This argument to a file access function is derived from $@ and then passed to " + callChain, + taintSource, "user input (" + taintCause + ")" diff --git a/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString-standard.qhelp b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString-standard.qhelp new file mode 100644 index 000000000..e4ce1eaf6 --- /dev/null +++ b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString-standard.qhelp @@ -0,0 +1,392 @@ + + +
    +

    Errors can occur when incorrect assumptions are made about the type of data being read. These assumptions may be violated, for example, when binary data has been read from a file instead of text from a user's terminal or the output of a process is piped to stdin. (See FIO14-C. Understand the difference between text mode and binary mode with file streams.) On some systems, it may also be possible to input a null byte (as well as other binary codes) from the keyboard.

    +

    Subclause 7.21.7.2 of the C Standard [ISO/IEC 9899:2011] says,

    +
    +

    The fgets function returns s if successful. If end-of-file is encountered and no characters have been read into the array, the contents of the array remain unchanged and a null pointer is returned.

    +
    +

    The wide-character function fgetws() has the same behavior. Therefore, if fgets() or fgetws() returns a non-null pointer, it is safe to assume that the array contains data. However, it is erroneous to assume that the array contains a nonempty string because the data may contain null characters.

    +
    +
    +

    This noncompliant code example attempts to remove the trailing newline (\n) from an input line. The fgets() function is typically used to read a newline-terminated line of input from a stream. It takes a size parameter for the destination buffer and copies, at most, size - 1 characters from a stream to a character array.

    + #include <stdio.h> +#include <string.h> + +enum { BUFFER_SIZE = 1024 }; + +void func(void) { + char buf[BUFFER_SIZE]; + + if (fgets(buf, sizeof(buf), stdin) == NULL) { + /* Handle error */ + } + buf[strlen(buf) - 1] = '\0'; +} +

    The strlen() function computes the length of a string by determining the number of characters that precede the terminating null character. A problem occurs if the first character read from the input by fgets() happens to be a null character. This may occur, for example, if a binary data file is read by the fgets() call [Lai 2006]. If the first character in buf is a null character, strlen(buf) returns 0, the expression strlen(buf) - 1 wraps around to a large positive value, and a write-outside-array-bounds error occurs.

    +
    +
    +

    This compliant solution uses strchr() to replace the newline character in the string if it exists:

    + #include <stdio.h> +#include <string.h> + +enum { BUFFER_SIZE = 1024 }; + +void func(void) { + char buf[BUFFER_SIZE]; + char *p; + + if (fgets(buf, sizeof(buf), stdin)) { + p = strchr(buf, '\n'); + if (p) { + *p = '\0'; + } + } else { + /* Handle error */ + } +} +
    +
    +

    Incorrectly assuming that character data has been read can result in an out-of-bounds memory write or other flawed logic.

    + + + + + + + + + + + + + + + + + + + +
    + Rule + + Severity + + Likelihood + + Remediation Cost + + Priority + + Level +
    + FIO37-C + + High + + Probable + + Medium + + P12 + + L1 +
    +
    +
    + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    + Tool + + Version + + Checker + + Description +
    + + Astrée + + + 20.10 + + + Supported: Astrée reports defects due to returned (empty) strings. +
    + + Axivion Bauhaus Suite + + + 7.2.0 + + CertC-FIO37 + +
    + + CodeSonar + + + 6.2p0 + + (general) + + Considers the possibility that + fgets() + and + fgetws() + may return empty strings (Warnings of various classes may be triggered depending on subsequent operations on those strings. For example, the noncompliant code example cited above would trigger a buffer underrun warning.) +
    + + Compass/ROSE + + + + + Could detect some violations of this rule (In particular, it could detect the noncompliant code example by searching for + fgets() + , followed by + strlen() - 1 + , which could be −1. The crux of this rule is that a string returned by + fgets() + could still be empty, because the first + char + is ' + \0 + '. There are probably other code examples that violate this guideline; they would need to be enumerated before ROSE could detect them.) +
    + + Helix QAC + + + 2022.1 + + C4911, C4912, C4913 + C++4911, C++4912, C++4913 + +
    + + LDRA tool suite + + + 9.7.1 + + 44 S + + Enhanced enforcement +
    + + Parasoft C/C++test + + + 2021.2 + + CERT_C-FIO37-a + + Avoid accessing arrays out of bounds +
    + + Polyspace Bug Finder + + + R2022a + + + CERT C: Rule FIO37-C + + + Checks for use of indeterminate string (rule fully covered) +
    + + PRQA QA-C++ + + + 4.4 + + 2844 + +
    +
    +
    +

    Search for vulnerabilities resulting from the violation of this rule on the CERT website.

    +
    +
    +

    Key here (explains table format and definitions)

    + + + + + + + + + + + + + + + + + + + + + + + +
    + Taxonomy + + Taxonomy item + + Relationship +
    + + CERT C Secure Coding Standard + + + + FIO14-C. Understand the difference between text mode and binary mode with file streams + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
    + + CERT C Secure Coding Standard + + + + FIO20-C. Avoid unintentional truncation when using fgets() or fgetws() + + + Prior to 2018-01-12: CERT: Unspecified Relationship +
    + + CWE 2.11 + + + + CWE-241 + + , Improper Handling of Unexpected Data Type + + 2017-07-05: CERT: Rule subset of CWE +
    +
    +
    +

    Key here for mapping notes

    +

    CWE-241 and FIO37-C

    +

    CWE-241 = Union( FIO37-C, list) where list =

    +
      +
    • Improper handling of unexpected data type that does not come from the fgets() function.
      • +
    +
    +
    + + + + + + + + + + + + + + + +
    + [ + + ISO/IEC 9899:2011 + + ] + + Subclause 7.21.7.2, "The + fgets + Function" + Subclause 7.29.3.2, "The + fgetws + Function" +
    + [ + + Lai 2006 + + ] + +
    + [ + + Seacord 2013 + + ] + + Chapter 2, "Strings" +
    +
    +     \ No newline at end of file diff --git a/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qhelp b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qhelp new file mode 100644 index 000000000..0ef132869 --- /dev/null +++ b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qhelp @@ -0,0 +1,18 @@ + + + + +

    This query implements the CERT-C rule FIO37-C:

    +
    +

    Do not assume that fgets() or fgetws() returns a nonempty string when successful

    +
    +     + + +
  • + CERT-C: + FIO37-C: Do not assume that fgets() or fgetws() returns a nonempty string when successful + . +
    • +     +     \ No newline at end of file diff --git a/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql new file mode 100644 index 000000000..54f555d7c --- /dev/null +++ b/c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql @@ -0,0 +1,48 @@ +/** + * @id c/cert/successful-fgets-or-fgetws-may-return-an-empty-string + * @name FIO37-C: Do not assume that fgets() or fgetws() returns a nonempty string when successful + * @description A string returned by fgets() or fegtws() might be empty. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/cert/id/fio37-c + * correctness + * external/cert/obligation/rule + */ + +import cpp +import codingstandards.c.cert +import codingstandards.cpp.FgetsErrorManagement +import codingstandards.cpp.Dereferenced +import semmle.code.cpp.dataflow.TaintTracking + +/* + * CFG nodes that follows a successful call to `fgets` + */ + +ControlFlowNode followsNonnullFgets(FgetsLikeCall fgets) { + exists(FgetsGuard guard | + //fgets.getLocation().getStartLine() = [60] and + fgets = guard.getFgetCall() and + ( + result = guard.getNonNullSuccessor() + or + result = followsNonnullFgets(fgets).getASuccessor() + ) + ) +} + +from Expr e, FgetsLikeCall fgets +where + not isExcluded(e, IO3Package::successfulFgetsOrFgetwsMayReturnAnEmptyStringQuery()) and + e = followsNonnullFgets(fgets) and + ( + e instanceof ArrayExpr + or + e instanceof PointerDereferenceExpr + ) and + not exists(GuardCondition guard | + guard.controls(e.getBasicBlock(), _) and guard = followsNonnullFgets(fgets) + ) +select e, "The string $@ could be empty when accessed at this location.", fgets.getBuffer(), + fgets.getBuffer().toString() diff --git a/c/cert/src/rules/FIO40-C/ResetStringsOnFgetsOrFgetwsFailure.ql b/c/cert/src/rules/FIO40-C/ResetStringsOnFgetsOrFgetwsFailure.ql index 30432797a..69fb92a15 100644 --- a/c/cert/src/rules/FIO40-C/ResetStringsOnFgetsOrFgetwsFailure.ql +++ b/c/cert/src/rules/FIO40-C/ResetStringsOnFgetsOrFgetwsFailure.ql @@ -58,9 +58,6 @@ class BuffAccessExpr extends Expr { // dereferenced expressions this instanceof DereferencedExpr or - // any array access `array[0]` - this = any(ArrayExpr ae).getArrayBase() - or // any parameter to a function this = any(FunctionCall fc).getAnArgument() } diff --git a/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar-standard.qhelp b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar-standard.qhelp new file mode 100644 index 000000000..458fbe3f7 --- /dev/null +++ b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar-standard.qhelp @@ -0,0 +1,33 @@ + + +
    +
      +
    • required
      • +
    • implementation
      • +
    • automated
      • +
    +
    + +
    +

    + ... +

    + +
    + +
    +

    + ... +

    +
    + + + + + +
    +
      +
    • ...
      • +
    +
    +     \ No newline at end of file diff --git a/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qhelp b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qhelp new file mode 100644 index 000000000..63fc3848b --- /dev/null +++ b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qhelp @@ -0,0 +1,18 @@ + + + + +

    This query implements the CERT-C rule STR37-C:

    +
    +

    Arguments to character-handling functions must be representable as an unsigned char

    +
    +     + + +
  • + CERT-C: + STR37-C: Arguments to character-handling functions must be representable as an unsigned char + . +
    • +     +     \ No newline at end of file diff --git a/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql new file mode 100644 index 000000000..cb742859c --- /dev/null +++ b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql @@ -0,0 +1,29 @@ +/** + * @id c/cert/to-character-handling-functions-representable-as-u-char + * @name STR37-C: Arguments to character-handling functions must be representable as an unsigned char + * @description Arguments that are not representable as an unsigned char may produce unpredictable + * program behavior. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/cert/id/str37-c + * correctness + * security + * external/cert/obligation/rule + */ + +import cpp +import codingstandards.c.cert +import codingstandards.cpp.CharFunctions + +from FunctionCall fc, Expr arg +where + not isExcluded(fc, Strings2Package::toCharacterHandlingFunctionsRepresentableAsUCharQuery()) and + // examine all impacted functions + fc.getTarget() instanceof CToOrIsCharFunction and + arg = fc.getArgument(0).getFullyConverted() and + // report on cases where either the explicit or implicit cast + // on the parameter type is not unsigned + not arg.(CStyleCast).getExpr().getType() instanceof UnsignedCharType +select fc, "$@ to character-handling function may not be representable as an unsigned char.", arg, + "Argument" diff --git a/c/cert/src/rules/FIO39-C/standard-example.c b/c/cert/src/rules/STR37-C/standard-example.c similarity index 100% rename from c/cert/src/rules/FIO39-C/standard-example.c rename to c/cert/src/rules/STR37-C/standard-example.c diff --git a/c/cert/test/qlpack.yml b/c/cert/test/qlpack.yml index dfe16b190..4ae74b3c8 100644 --- a/c/cert/test/qlpack.yml +++ b/c/cert/test/qlpack.yml @@ -1,4 +1,4 @@ name: cert-c-coding-standards-tests -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: cert-c-coding-standards extractor: cpp \ No newline at end of file diff --git a/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected b/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected new file mode 100644 index 000000000..c9252151d --- /dev/null +++ b/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.expected @@ -0,0 +1,40 @@ +edges +| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | (const char *)... | +| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | file_name | +| test.c:20:15:20:23 | array to pointer conversion | test.c:21:8:21:16 | file_name indirection | +| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | (const char *)... | +| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name | +| test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name indirection | +| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | (const char *)... | +| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name | +| test.c:20:15:20:23 | scanf output argument | test.c:21:8:21:16 | file_name indirection | +| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | (LPCTSTR)... | +| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | file_name | +| test.c:45:15:45:23 | array to pointer conversion | test.c:46:29:46:37 | file_name indirection | +| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | (LPCTSTR)... | +| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name | +| test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name indirection | +| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | (LPCTSTR)... | +| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name | +| test.c:45:15:45:23 | scanf output argument | test.c:46:29:46:37 | file_name indirection | +subpaths +nodes +| test.c:20:15:20:23 | array to pointer conversion | semmle.label | array to pointer conversion | +| test.c:20:15:20:23 | file_name | semmle.label | file_name | +| test.c:20:15:20:23 | scanf output argument | semmle.label | scanf output argument | +| test.c:21:8:21:16 | (const char *)... | semmle.label | (const char *)... | +| test.c:21:8:21:16 | (const char *)... | semmle.label | (const char *)... | +| test.c:21:8:21:16 | file_name | semmle.label | file_name | +| test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection | +| test.c:21:8:21:16 | file_name indirection | semmle.label | file_name indirection | +| test.c:45:15:45:23 | array to pointer conversion | semmle.label | array to pointer conversion | +| test.c:45:15:45:23 | file_name | semmle.label | file_name | +| test.c:45:15:45:23 | scanf output argument | semmle.label | scanf output argument | +| test.c:46:29:46:37 | (LPCTSTR)... | semmle.label | (LPCTSTR)... | +| test.c:46:29:46:37 | (LPCTSTR)... | semmle.label | (LPCTSTR)... | +| test.c:46:29:46:37 | file_name | semmle.label | file_name | +| test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection | +| test.c:46:29:46:37 | file_name indirection | semmle.label | file_name indirection | +#select +| test.c:21:8:21:16 | file_name | test.c:20:15:20:23 | file_name | test.c:21:8:21:16 | file_name | This argument to a file access function is derived from $@ and then passed to func(file_name), which calls fopen((unnamed parameter 0)) | test.c:20:15:20:23 | file_name | user input (scanf) | +| test.c:46:29:46:37 | file_name | test.c:45:15:45:23 | file_name | test.c:46:29:46:37 | file_name | This argument to a file access function is derived from $@ and then passed to CreateFile(lpFileName) | test.c:45:15:45:23 | file_name | user input (scanf) | diff --git a/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qlref b/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qlref new file mode 100644 index 000000000..a6c5fea94 --- /dev/null +++ b/c/cert/test/rules/FIO32-C/DoNotPerformFileOperationsOnDevices.qlref @@ -0,0 +1 @@ +rules/FIO32-C/DoNotPerformFileOperationsOnDevices.ql \ No newline at end of file diff --git a/c/cert/test/rules/FIO32-C/test.c b/c/cert/test/rules/FIO32-C/test.c new file mode 100644 index 000000000..063cd287d --- /dev/null +++ b/c/cert/test/rules/FIO32-C/test.c @@ -0,0 +1,50 @@ +#include +#include +#include + +void func(const char *file_name) { + FILE *file; + if ((file = fopen(file_name, "wb")) == NULL) { + /* Handle error */ + } + + /* Operate on the file */ + + if (fclose(file) == EOF) { + /* Handle error */ + } +} + +int main(int argc, char **argv) { + char file_name[20]; + scanf("%s", file_name); + func(file_name); // NON_COMPLIANT + func("file_name"); // COMPLIANT +} + +// --- Windows --- + +typedef void *HANDLE; +#define GENERIC_READ 0x80000000 +#define GENERIC_WRITE 0x40000000 +#define OPEN_EXISTING 3 +#define FILE_ATTRIBUTE_NORMAL 0x00000080 +typedef const char *LPCTSTR; +typedef unsigned long DWORD; +typedef struct _SECURITY_ATTRIBUTES { +} * LPSECURITY_ATTRIBUTES; +typedef bool BOOL; +HANDLE CreateFile(LPCTSTR lpFileName, DWORD dwDesiredAccess, DWORD dwShareMode, + LPSECURITY_ATTRIBUTES lpSecurityAttributes, + DWORD dwCreationDisposition, DWORD dwFlagsAndAttributes, + HANDLE hTemplateFile); +BOOL CloseHandle(HANDLE hObject); + +void funcWin() { + char file_name[20]; + scanf("%s", file_name); + HANDLE hFile = CreateFile(file_name, // NON_COMPLIANT + GENERIC_READ | GENERIC_WRITE, 0, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + CloseHandle(hFile); +} \ No newline at end of file diff --git a/c/cert/test/rules/FIO34-C/EndOfFileCheckPortability.expected b/c/cert/test/rules/FIO34-C/EndOfFileCheckPortability.expected index 38df60ea2..7ba0133dc 100644 --- a/c/cert/test/rules/FIO34-C/EndOfFileCheckPortability.expected +++ b/c/cert/test/rules/FIO34-C/EndOfFileCheckPortability.expected @@ -1,6 +1,5 @@ | test.c:11:12:11:19 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | | test.c:19:12:19:19 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | -| test.c:77:13:77:22 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | | test.c:96:12:96:19 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | | test.c:101:10:102:15 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | | test.c:116:12:116:19 | ... != ... | This check is only portable to platforms where type `char` is less wide than type `int`. | diff --git a/c/cert/test/rules/FIO34-C/test.c b/c/cert/test/rules/FIO34-C/test.c index 7bf1119c0..0faf978cd 100644 --- a/c/cert/test/rules/FIO34-C/test.c +++ b/c/cert/test/rules/FIO34-C/test.c @@ -74,7 +74,7 @@ void f4(void) { size_t i = 0; while ((wc = getwc(stdin)) != L'\n' // COMPLIANT - && wc != WEOF) { // NON_PORTABLE + && wc != WEOF) { // PORTABLE if (i < BUFFER_SIZE - 1) { buf[i++] = wc; } diff --git a/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.expected b/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.expected new file mode 100644 index 000000000..36b28900c --- /dev/null +++ b/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.expected @@ -0,0 +1 @@ +| test.c:13:3:13:22 | access to array | The string $@ could be empty when accessed at this location. | test.c:8:13:8:15 | buf | buf | diff --git a/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qlref b/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qlref new file mode 100644 index 000000000..a831b3dcf --- /dev/null +++ b/c/cert/test/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.qlref @@ -0,0 +1 @@ +rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql \ No newline at end of file diff --git a/c/cert/test/rules/FIO37-C/test.c b/c/cert/test/rules/FIO37-C/test.c new file mode 100644 index 000000000..5c59d855a --- /dev/null +++ b/c/cert/test/rules/FIO37-C/test.c @@ -0,0 +1,56 @@ +#include +#include +#include + +char buf[1024]; + +void f1() { + if (fgets(buf, sizeof(buf), stdin) == NULL) { + /*null*/ + return; + } + /*notnull*/ + buf[strlen(buf) - 1] = '\0'; // NON_COMPLIANT + return; +} + +void f2() { + char *p; + if (fgets(buf, sizeof(buf), stdin)) { + /*notnull*/ + p = strchr(buf, '\n'); + if (p) { + *p = '\0'; // COMPLIANT + } + } + return; +} + +static inline bool strends(const char *str, const char *postfix) { + if (strlen(str) < strlen(postfix)) + return false; + + return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0; +} +void f3() { + if (fgets(buf, sizeof(buf), stdin)) { + /*notnull*/ + if (strends(buf, "\n")) { + buf[strlen(buf) - 1] = '\0'; // COMPLIANT + } + } + return; +} + +void f4() { + char *p; + if (fgets(buf, sizeof(buf), stdin) == NULL) { + return; + } + p = strchr(buf, '\n'); + if (p) { + *p = '\0'; // COMPLIANT + } + + return; +} diff --git a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected new file mode 100644 index 000000000..b655289f4 --- /dev/null +++ b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected @@ -0,0 +1,28 @@ +| test.c:7:3:7:9 | call to isalnum | $@ to character-handling function may not be representable as an unsigned char. | test.c:7:11:7:12 | (int)... | Argument | +| test.c:8:3:8:13 | call to isalpha | $@ to character-handling function may not be representable as an unsigned char. | test.c:8:11:8:12 | (int)... | Argument | +| test.c:10:3:10:9 | call to isblank | $@ to character-handling function may not be representable as an unsigned char. | test.c:10:11:10:12 | (int)... | Argument | +| test.c:11:3:11:9 | call to iscntrl | $@ to character-handling function may not be representable as an unsigned char. | test.c:11:11:11:12 | (int)... | Argument | +| test.c:12:3:12:13 | call to isdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:12:11:12:12 | (int)... | Argument | +| test.c:13:3:13:13 | call to isgraph | $@ to character-handling function may not be representable as an unsigned char. | test.c:13:11:13:12 | (int)... | Argument | +| test.c:14:3:14:13 | call to islower | $@ to character-handling function may not be representable as an unsigned char. | test.c:14:11:14:12 | (int)... | Argument | +| test.c:15:3:15:13 | call to isprint | $@ to character-handling function may not be representable as an unsigned char. | test.c:15:11:15:12 | (int)... | Argument | +| test.c:16:3:16:9 | call to ispunct | $@ to character-handling function may not be representable as an unsigned char. | test.c:16:11:16:12 | (int)... | Argument | +| test.c:17:3:17:13 | call to __isspace | $@ to character-handling function may not be representable as an unsigned char. | test.c:17:11:17:12 | (int)... | Argument | +| test.c:18:3:18:13 | call to isupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:18:11:18:12 | (int)... | Argument | +| test.c:19:3:19:10 | call to isxdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:19:12:19:13 | (int)... | Argument | +| test.c:21:3:21:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:21:11:21:12 | (int)... | Argument | +| test.c:22:3:22:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:22:11:22:12 | (int)... | Argument | +| test.c:70:3:70:9 | call to isalnum | $@ to character-handling function may not be representable as an unsigned char. | test.c:70:11:70:11 | t | Argument | +| test.c:71:3:71:12 | call to isalpha | $@ to character-handling function may not be representable as an unsigned char. | test.c:71:11:71:11 | t | Argument | +| test.c:73:3:73:9 | call to isblank | $@ to character-handling function may not be representable as an unsigned char. | test.c:73:11:73:11 | t | Argument | +| test.c:74:3:74:9 | call to iscntrl | $@ to character-handling function may not be representable as an unsigned char. | test.c:74:11:74:11 | t | Argument | +| test.c:75:3:75:12 | call to isdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:75:11:75:11 | t | Argument | +| test.c:76:3:76:12 | call to isgraph | $@ to character-handling function may not be representable as an unsigned char. | test.c:76:11:76:11 | t | Argument | +| test.c:77:3:77:12 | call to islower | $@ to character-handling function may not be representable as an unsigned char. | test.c:77:11:77:11 | t | Argument | +| test.c:78:3:78:12 | call to isprint | $@ to character-handling function may not be representable as an unsigned char. | test.c:78:11:78:11 | t | Argument | +| test.c:79:3:79:9 | call to ispunct | $@ to character-handling function may not be representable as an unsigned char. | test.c:79:11:79:11 | t | Argument | +| test.c:80:3:80:12 | call to __isspace | $@ to character-handling function may not be representable as an unsigned char. | test.c:80:11:80:11 | t | Argument | +| test.c:81:3:81:12 | call to isupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:81:11:81:11 | t | Argument | +| test.c:82:3:82:10 | call to isxdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:82:12:82:12 | t | Argument | +| test.c:84:3:84:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:84:11:84:11 | t | Argument | +| test.c:85:3:85:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:85:11:85:11 | t | Argument | diff --git a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qlref b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qlref new file mode 100644 index 000000000..d796e1653 --- /dev/null +++ b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.qlref @@ -0,0 +1 @@ +rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql \ No newline at end of file diff --git a/c/cert/test/rules/STR37-C/test.c b/c/cert/test/rules/STR37-C/test.c new file mode 100644 index 000000000..639e4b8f0 --- /dev/null +++ b/c/cert/test/rules/STR37-C/test.c @@ -0,0 +1,86 @@ +#include +#include + +void f1() { + char *t; + + isalnum(*t); // NON_COMPLIANT + isalpha(*t); // NON_COMPLIANT + // isascii(*t); // Not part of the C Standard + isblank(*t); // NON_COMPLIANT + iscntrl(*t); // NON_COMPLIANT + isdigit(*t); // NON_COMPLIANT + isgraph(*t); // NON_COMPLIANT + islower(*t); // NON_COMPLIANT + isprint(*t); // NON_COMPLIANT + ispunct(*t); // NON_COMPLIANT + isspace(*t); // NON_COMPLIANT + isupper(*t); // NON_COMPLIANT + isxdigit(*t); // NON_COMPLIANT + // toascii(i); // Not part of the C Standard + toupper(*t); // NON_COMPLIANT + tolower(*t); // NON_COMPLIANT +} + +void f2() { + unsigned char *t; + + isalnum(*t); // COMPLIANT + isalpha(*t); // COMPLIANT + // isascii(*t); // Not part of the C Standard + isblank(*t); // COMPLIANT + iscntrl(*t); // COMPLIANT + isdigit(*t); // COMPLIANT + isgraph(*t); // COMPLIANT + islower(*t); // COMPLIANT + isprint(*t); // COMPLIANT + ispunct(*t); // COMPLIANT + isspace(*t); // COMPLIANT + isupper(*t); // COMPLIANT + isxdigit(*t); // COMPLIANT + // toascii(i); // Not part of the C Standard + toupper(*t); // COMPLIANT + tolower(*t); // COMPLIANT +} + +void f3() { + char *t; + + isalnum((unsigned char)*t); // COMPLIANT + isalpha((unsigned char)*t); // COMPLIANT + // isascii((unsigned char*)*t); // Not part of the C Standard + isblank((unsigned char)*t); // COMPLIANT + iscntrl((unsigned char)*t); // COMPLIANT + isdigit((unsigned char)*t); // COMPLIANT + isgraph((unsigned char)*t); // COMPLIANT + islower((unsigned char)*t); // COMPLIANT + isprint((unsigned char)*t); // COMPLIANT + ispunct((unsigned char)*t); // COMPLIANT + isspace((unsigned char)*t); // COMPLIANT + isupper((unsigned char)*t); // COMPLIANT + isxdigit((unsigned char)*t); // COMPLIANT + // toascii((unsigned int) i); // Not part of the C Standard + toupper((unsigned char)*t); // COMPLIANT + tolower((unsigned char)*t); // COMPLIANT +} + +void f4() { + int t; + + isalnum(t); // NON_COMPLIANT + isalpha(t); // NON_COMPLIANT + // isascii(t); // Not part of the C Standard + isblank(t); // NON_COMPLIANT + iscntrl(t); // NON_COMPLIANT + isdigit(t); // NON_COMPLIANT + isgraph(t); // NON_COMPLIANT + islower(t); // NON_COMPLIANT + isprint(t); // NON_COMPLIANT + ispunct(t); // NON_COMPLIANT + isspace(t); // NON_COMPLIANT + isupper(t); // NON_COMPLIANT + isxdigit(t); // NON_COMPLIANT + // toascii(i); // Not part of the C Standard + toupper(t); // NON_COMPLIANT + tolower(t); // NON_COMPLIANT +} diff --git a/c/common/src/qlpack.yml b/c/common/src/qlpack.yml index ddab9cafc..586aa9755 100644 --- a/c/common/src/qlpack.yml +++ b/c/common/src/qlpack.yml @@ -1,3 +1,3 @@ name: common-c-coding-standards -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: common-cpp-coding-standards diff --git a/c/common/test/expr/FullExpr.expected b/c/common/test/library/expr/FullExpr.expected similarity index 100% rename from c/common/test/expr/FullExpr.expected rename to c/common/test/library/expr/FullExpr.expected diff --git a/c/common/test/expr/FullExpr.ql b/c/common/test/library/expr/FullExpr.ql similarity index 100% rename from c/common/test/expr/FullExpr.ql rename to c/common/test/library/expr/FullExpr.ql diff --git a/c/common/test/expr/fullexpr.c b/c/common/test/library/expr/fullexpr.c similarity index 100% rename from c/common/test/expr/fullexpr.c rename to c/common/test/library/expr/fullexpr.c diff --git a/c/common/test/library/fgetserrormanagement/FgetsGuard.expected b/c/common/test/library/fgetserrormanagement/FgetsGuard.expected new file mode 100644 index 000000000..5dd4ca1f6 --- /dev/null +++ b/c/common/test/library/fgetserrormanagement/FgetsGuard.expected @@ -0,0 +1,20 @@ +| test.c:9:10:11:3 | { ... } | test.c:7:38:9:3 | { ... } | +| test.c:16:39:18:3 | { ... } | test.c:18:10:20:3 | { ... } | +| test.c:25:43:27:3 | { ... } | test.c:27:10:29:3 | { ... } | +| test.c:36:10:38:3 | { ... } | test.c:34:43:36:3 | { ... } | +| test.c:45:10:47:3 | { ... } | test.c:43:46:45:3 | { ... } | +| test.c:53:10:55:3 | { ... } | test.c:51:49:53:3 | { ... } | +| test.c:66:3:66:13 | return ... | test.c:60:49:62:3 | { ... } | +| test.c:72:10:74:3 | { ... } | test.c:70:48:72:3 | { ... } | +| test.c:83:3:83:13 | return ... | test.c:79:48:81:3 | { ... } | +| test.c:87:46:89:3 | { ... } | test.c:89:10:91:3 | { ... } | +| test.c:98:13:100:3 | { ... } | test.c:100:10:102:3 | { ... } | +| test.c:111:10:113:3 | { ... } | test.c:109:14:111:3 | { ... } | +| test.c:118:66:121:3 | { ... } | test.c:118:66:121:3 | { ... } | +| test.c:128:63:131:3 | { ... } | test.c:128:63:131:3 | { ... } | +| test.c:140:10:142:3 | { ... } | test.c:138:54:140:3 | { ... } | +| test.c:147:54:149:3 | { ... } | test.c:149:10:151:3 | { ... } | +| test.c:158:10:161:3 | { ... } | test.c:158:10:161:3 | { ... } | +| test.c:168:10:171:3 | { ... } | test.c:168:10:171:3 | { ... } | +| test.c:176:54:179:3 | { ... } | test.c:179:10:182:3 | { ... } | +| test.c:190:10:193:3 | { ... } | test.c:187:54:190:3 | { ... } | diff --git a/c/common/test/library/fgetserrormanagement/FgetsGuard.ql b/c/common/test/library/fgetserrormanagement/FgetsGuard.ql new file mode 100644 index 000000000..f5dbde275 --- /dev/null +++ b/c/common/test/library/fgetserrormanagement/FgetsGuard.ql @@ -0,0 +1,5 @@ +import cpp +import codingstandards.cpp.FgetsErrorManagement + +from FgetsGuard e +select e.getNullSuccessor(), e.getNonNullSuccessor() diff --git a/c/common/test/library/fgetserrormanagement/test.c b/c/common/test/library/fgetserrormanagement/test.c new file mode 100644 index 000000000..13ba91117 --- /dev/null +++ b/c/common/test/library/fgetserrormanagement/test.c @@ -0,0 +1,195 @@ +#include +#include + +char buf[1024]; + +void f0(FILE *file) { + if (fgets(buf, sizeof(buf), file)) { + /*notNull*/ + } else { + /*null*/ + } + return; +} + +void f1(FILE *file) { + if (!fgets(buf, sizeof(buf), file)) { + /*null*/ + } else { + /*notNull*/ + } + return; +} + +void f2(FILE *file) { + if (fgets(buf, sizeof(buf), file) == 0) { + /*null*/ + } else { + /*notNull*/ + } + return; +} + +void f3(FILE *file) { + if (fgets(buf, sizeof(buf), file) != 0) { + /*notNull*/ + } else { + /*null*/ + } + return; +} + +void f4(FILE *file) { + if (!(fgets(buf, sizeof(buf), file) == 0)) { + /*notNull*/ + } else { + /*null*/ + } + return; +} +char *f4a(FILE *file) { + if (!(fgets(buf, sizeof(buf), file) == NULL)) { + /*notNull*/ + } else { + /*null*/ + } + return buf; // NON_COMPLIANT +} + +char *f4b(FILE *file) { + if (!(fgets(buf, sizeof(buf), file) == NULL)) { + /*notNull*/ + } + /*notNull*/ + /*null*/ + + return buf; // NON_COMPLIANT +} + +char *f4c(FILE *file) { + if ((fgets(buf, sizeof(buf), file) != NULL)) { + /*notNull*/ + } else { + /*null*/ + } + return buf; // NON_COMPLIANT +} + +char *f4d(FILE *file) { + if ((fgets(buf, sizeof(buf), file) != NULL)) { + /*notNull*/ + } + /*null*/ + return buf; // NON_COMPLIANT +} + +void f5(FILE *file) { + if (!(fgets(buf, sizeof(buf), file) != 0)) { + /*null*/ + } else { + /*notNull*/ + } + return; +} + +void f6(FILE *file) { + char *ret = fgets(buf, sizeof(buf), file); + bool flag = (ret == NULL); + if (flag) { + /*null*/ + } else { + /*notNull*/ + } + return; +} + +void f7(FILE *file) { + char *ret = fgets(buf, sizeof(buf), file); + bool flag = (ret == NULL); + if (!flag) { + /*notNull*/ + } else { + /*null*/ + } + return; +} + +void f8(FILE *file, bool cond) { + if (fgets(buf, sizeof(buf), file) == NULL || /*notNull*/ cond) { + /*null*/ + /*notNull*/ + } else { + /*notNull*/ + } + return; +} + +void f9a(FILE *file, bool cond) { + if (fgets(buf, sizeof(buf), file) != NULL || /*null*/ cond) { + /*null*/ + /*notNull*/ + } else { + /*null*/ + } + return; +} + +void f9b(FILE *file, bool cond) { + if (cond || fgets(buf, sizeof(buf), file) != NULL) { + /*notNull*/ + } else { + /*null*/ + } + return; +} + +void f9c(FILE *file, bool cond) { + if (cond || fgets(buf, sizeof(buf), file) == NULL) { + /*null*/ + } else { + /*notNnull*/ + } + return; +} + +void f10(FILE *file, bool cond) { + if (fgets(buf, sizeof(buf), file) == NULL && /*null*/ cond) { + /*null*/ + } else { + /*null*/ + /*notNull*/ + } + return; +} + +void f11a(FILE *file, bool cond) { + if (fgets(buf, sizeof(buf), file) != NULL && /*notNull*/ cond) { + /*notNull*/ + } else { + /*null*/ + /*notNull*/ + } + return; +} + +void f11b(FILE *file, bool cond) { + if (cond && fgets(buf, sizeof(buf), file) == NULL) { + /*null*/ + fprintf(stderr, "Read error or end of file.\n"); + } else { + /*notNull*/ + fprintf(stderr, "Read OK.\n"); + } + return; +} + +void f11c(FILE *file, bool cond) { + if (cond && fgets(buf, sizeof(buf), file) != NULL) { + /*notNull*/ + fprintf(stderr, "Read error or end of file.\n"); + } else { + /*null*/ + fprintf(stderr, "Read OK.\n"); + } + return; +} \ No newline at end of file diff --git a/c/common/test/qlpack.yml b/c/common/test/qlpack.yml index 35fc480d2..123450b5e 100644 --- a/c/common/test/qlpack.yml +++ b/c/common/test/qlpack.yml @@ -1,4 +1,4 @@ name: common-c-coding-standards-tests -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: common-c-coding-standards extractor: cpp diff --git a/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.expected b/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.expected deleted file mode 100644 index e844aafb0..000000000 --- a/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.expected +++ /dev/null @@ -1,4 +0,0 @@ -| test.c:9:1:9:22 | #define MACROFIVE(X) #X | Macro definition uses the # or ## operator. | -| test.c:11:1:11:26 | #define MACROSIX(X,Y) X ## Y | Macro definition uses the # or ## operator. | -| test.c:13:1:13:29 | #define MACROSEVEN "##'" #"#" | Macro definition uses the # or ## operator. | -| test.c:15:1:15:28 | #define MACROEIGHT '##' #"#" | Macro definition uses the # or ## operator. | diff --git a/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.ql b/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.ql deleted file mode 100644 index d0ead0b28..000000000 --- a/c/common/test/rules/hashoperatorsused/HashOperatorsUsed.ql +++ /dev/null @@ -1,2 +0,0 @@ -// GENERATED FILE - DO NOT MODIFY -import codingstandards.cpp.rules.hashoperatorsused.HashOperatorsUsed diff --git a/c/common/test/rules/hashoperatorsused/test.c b/c/common/test/rules/hashoperatorsused/test.c deleted file mode 100644 index 422bde164..000000000 --- a/c/common/test/rules/hashoperatorsused/test.c +++ /dev/null @@ -1,19 +0,0 @@ -#define MACROONE 1 // COMPLIANT - -#define MACROTWO '#' // COMPLIANT - -#define MACROTHREE "##" // COMPLIANT - -#define MACROFOUR "##" + "#" // COMPLIANT - -#define MACROFIVE(X) #X // NON_COMPLIANT - -#define MACROSIX(X, Y) X##Y // NON_COMPLIANT - -#define MACROSEVEN "##'" #"#" // NON_COMPLIANT - -#define MACROEIGHT '##' #"#" // NON_COMPLIANT - -#define MACRONINE "##\"\"" + "#" // COMPLIANT - -#define MACROTEN "##\"\"'" + "#" // COMPLIANT \ No newline at end of file diff --git a/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.expected b/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.expected new file mode 100644 index 000000000..8d72392ed --- /dev/null +++ b/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.expected @@ -0,0 +1,4 @@ +| headers/test3.h:0:0:0:0 | headers/test3.h | Header file test3.h is missing expected include guard. | headers/test3.h:0:0:0:0 | headers/test3.h | | +| headers/test4.h:0:0:0:0 | headers/test4.h | Header file test4.h is missing expected include guard. | headers/test4.h:0:0:0:0 | headers/test4.h | | +| headers/test5.h:0:0:0:0 | headers/test5.h | Header file test5.h is missing expected include guard. | headers/test5.h:0:0:0:0 | headers/test5.h | | +| headers/test7.h:0:0:0:0 | headers/test7.h | Header file test7.h is never included by reusing the include guard used by $@. | headers/test6.h:0:0:0:0 | headers/test6.h | include guard | diff --git a/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql b/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql new file mode 100644 index 000000000..2fcfddeca --- /dev/null +++ b/c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql @@ -0,0 +1,2 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.includeguardsnotused.IncludeGuardsNotUsed diff --git a/cpp/autosar/test/rules/M16-2-3/headers/test1.hpp b/c/common/test/rules/includeguardsnotused/headers/test1.h similarity index 100% rename from cpp/autosar/test/rules/M16-2-3/headers/test1.hpp rename to c/common/test/rules/includeguardsnotused/headers/test1.h diff --git a/cpp/autosar/test/rules/M16-2-3/headers/test2.hpp b/c/common/test/rules/includeguardsnotused/headers/test2.h similarity index 100% rename from cpp/autosar/test/rules/M16-2-3/headers/test2.hpp rename to c/common/test/rules/includeguardsnotused/headers/test2.h diff --git a/cpp/autosar/test/rules/M16-2-3/headers/test3.hpp b/c/common/test/rules/includeguardsnotused/headers/test3.h similarity index 100% rename from cpp/autosar/test/rules/M16-2-3/headers/test3.hpp rename to c/common/test/rules/includeguardsnotused/headers/test3.h diff --git a/cpp/autosar/test/rules/M16-2-3/headers/test4.hpp b/c/common/test/rules/includeguardsnotused/headers/test4.h similarity index 100% rename from cpp/autosar/test/rules/M16-2-3/headers/test4.hpp rename to c/common/test/rules/includeguardsnotused/headers/test4.h diff --git a/cpp/autosar/test/rules/M16-2-3/headers/test5.hpp b/c/common/test/rules/includeguardsnotused/headers/test5.h similarity index 100% rename from cpp/autosar/test/rules/M16-2-3/headers/test5.hpp rename to c/common/test/rules/includeguardsnotused/headers/test5.h diff --git a/c/common/test/rules/includeguardsnotused/headers/test6.h b/c/common/test/rules/includeguardsnotused/headers/test6.h new file mode 100644 index 000000000..48dc2ab42 --- /dev/null +++ b/c/common/test/rules/includeguardsnotused/headers/test6.h @@ -0,0 +1,5 @@ +// NON_COMPLIANT +#ifndef HEADER_SIX +#define HEADER_SIX +int g5; +#endif \ No newline at end of file diff --git a/c/common/test/rules/includeguardsnotused/headers/test7.h b/c/common/test/rules/includeguardsnotused/headers/test7.h new file mode 100644 index 000000000..48dc2ab42 --- /dev/null +++ b/c/common/test/rules/includeguardsnotused/headers/test7.h @@ -0,0 +1,5 @@ +// NON_COMPLIANT +#ifndef HEADER_SIX +#define HEADER_SIX +int g5; +#endif \ No newline at end of file diff --git a/c/common/test/rules/includeguardsnotused/test.c b/c/common/test/rules/includeguardsnotused/test.c new file mode 100644 index 000000000..8c63904fc --- /dev/null +++ b/c/common/test/rules/includeguardsnotused/test.c @@ -0,0 +1,14 @@ +#include "headers/test1.h" + +#include "headers/test2.h" + +#include "headers/test3.h" //NON_COMPLIANT + +#include "headers/test4.h" //NON_COMPLIANT +#define HEADER_FOUR + +#include "headers/test5.h" //NON_COMPLIANT + +#include "headers/test6.h" //NON_COMPLIANT - non unique and reported in alert for the next + +#include "headers/test7.h" //NON_COMPLIANT - non unique \ No newline at end of file diff --git a/c/misra/src/qlpack.yml b/c/misra/src/qlpack.yml index 2925a83ae..01973953c 100644 --- a/c/misra/src/qlpack.yml +++ b/c/misra/src/qlpack.yml @@ -1,4 +1,4 @@ name: misra-c-coding-standards -version: 2.3.0 +version: 2.4.0 suites: codeql-suites libraryPathDependencies: common-c-coding-standards diff --git a/c/misra/src/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.ql b/c/misra/src/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.ql new file mode 100644 index 000000000..6ea7aa0a1 --- /dev/null +++ b/c/misra/src/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.ql @@ -0,0 +1,26 @@ +/** + * @id c/misra/more-than-one-hash-operator-in-macro-definition + * @name RULE-20-11: A macro parameter immediately following a # operator shall not immediately be followed by a ## + * @description The order of evaluation for the '#' and '##' operators may differ between compilers, + * which can cause unexpected behaviour if more than one '#' or '##' operator is used. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-20-11 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Macro + +from Macro m +where + not isExcluded(m, Preprocessor2Package::moreThanOneHashOperatorInMacroDefinitionQuery()) and + exists(StringizingOperator one, TokenPastingOperator two | + one.getMacro() = m and + two.getMacro() = m and + one.getOffset() < two.getOffset() + ) +select m, "Macro definition uses an # operator followed by a ## operator." diff --git a/c/misra/src/rules/RULE-20-11/standard-example.c b/c/misra/src/rules/RULE-20-11/standard-example.c new file mode 100644 index 000000000..97d0f68ba --- /dev/null +++ b/c/misra/src/rules/RULE-20-11/standard-example.c @@ -0,0 +1,3 @@ +#define A(x) #x /* Compliant */ +#define B(x, y) x##y /* Compliant */ +#define C(x, y) #x##y /* Non-compliant */ \ No newline at end of file diff --git a/c/misra/src/rules/RULE-20-12/MacroParameterUsedAsHashOperand.ql b/c/misra/src/rules/RULE-20-12/MacroParameterUsedAsHashOperand.ql new file mode 100644 index 000000000..779c14176 --- /dev/null +++ b/c/misra/src/rules/RULE-20-12/MacroParameterUsedAsHashOperand.ql @@ -0,0 +1,39 @@ +/** + * @id c/misra/macro-parameter-used-as-hash-operand + * @name RULE-20-12: A macro parameter used as an operand to the # or ## operators shall only be used as an operand to these operators + * @description A macro parameter used in different contexts such as: 1) an operand to the # or ## + * operators where it is not expanded, versus 2) elsewhere where it is expanded, makes + * code more difficult to understand. + * @kind problem + * @precision high + * @problem.severity warning + * @tags external/misra/id/rule-20-12 + * maintainability + * readability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Macro + +from FunctionLikeMacro m, MacroInvocation mi, int i, string expanded, string param +where + not isExcluded(m, Preprocessor2Package::macroParameterUsedAsHashOperandQuery()) and + mi = m.getAnInvocation() and + param = m.getParameter(i) and + ( + exists(TokenPastingOperator op | op.getMacro() = m and op.getOperand() = param) + or + exists(StringizingOperator op | op.getMacro() = m and op.getOperand() = param) + ) and + // An expansion that is equal to "" means the expansion is not used and is optimized away by EDG. This happens when the expanded argument is an operand to `#` or `##`. + // This check ensure there is an expansion that is used. + expanded = mi.getExpandedArgument(i) and + not expanded = "" and + exists(Macro furtherExpandedMacro | + mi.getUnexpandedArgument(i).matches(furtherExpandedMacro.getName() + "%") + ) +select m, + "Macro " + m.getName() + " contains use of parameter " + m.getParameter(i) + + " used in multiple contexts." diff --git a/c/misra/src/rules/RULE-20-12/standard-example.c b/c/misra/src/rules/RULE-20-12/standard-example.c new file mode 100644 index 000000000..bcfaea6e2 --- /dev/null +++ b/c/misra/src/rules/RULE-20-12/standard-example.c @@ -0,0 +1,7 @@ +#define AA 0xffff +#define BB(x) (x) + wow##x /* Non-compliant */ +void f(void) { + int32_t wowAA = 0; + /* Expands as wowAA = ( 0xffff ) + wowAA; */ + wowAA = BB(AA); +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-20-5/UndefShouldNotBeUsed.ql b/c/misra/src/rules/RULE-20-5/UndefShouldNotBeUsed.ql new file mode 100644 index 000000000..c253c795e --- /dev/null +++ b/c/misra/src/rules/RULE-20-5/UndefShouldNotBeUsed.ql @@ -0,0 +1,19 @@ +/** + * @id c/misra/undef-should-not-be-used + * @name RULE-20-5: #undef should not be used + * @description Using the #undef preprocessor directive makes code more difficult to understand. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-20-5 + * maintainability + * readability + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.c.misra + +from PreprocessorUndef d +where not isExcluded(d, Preprocessor2Package::undefShouldNotBeUsedQuery()) +select d, "Use of undef found." diff --git a/c/misra/src/rules/RULE-20-5/standard-example.c b/c/misra/src/rules/RULE-20-5/standard-example.c new file mode 100644 index 000000000..ad7783899 --- /dev/null +++ b/c/misra/src/rules/RULE-20-5/standard-example.c @@ -0,0 +1,7 @@ +#define QUALIFIER volatile +#undef QUALIFIER /* Non-compliant */ +void f(QUALIFIER int32_t p) { + while (p != 0) { + ; /* Wait... */ + } +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.ql b/c/misra/src/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.ql new file mode 100644 index 000000000..877fbea9a --- /dev/null +++ b/c/misra/src/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.ql @@ -0,0 +1,68 @@ +/** + * @id c/misra/file-open-for-read-and-write-on-different-streams + * @name RULE-22-3: The same file shall not be open for read and write access at the same time on different streams + * @description Accessing the same file for read and write from different streams is undefined + * behavior. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-3 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.standardlibrary.FileAccess +import semmle.code.cpp.dataflow.DataFlow +import semmle.code.cpp.valuenumbering.GlobalValueNumbering +import semmle.code.cpp.controlflow.SubBasicBlocks + +/** + * Models calls to `fopen` with different read/write modes + */ +class FOpenSBB extends SubBasicBlockCutNode { + FOpenSBB() { + this instanceof FOpenCall or + this instanceof FileCloseFunctionCall + } +} + +SubBasicBlock followsOpen(FOpenCall fopen) { + result = fopen + or + exists(SubBasicBlock mid | + result = mid.getASuccessor() and + mid = followsOpen(fopen) and + // stop recursion when the first stream is closed + not DataFlow::localExprFlow(fopen, result.(FileCloseFunctionCall).getFileExpr()) + ) +} + +class MatchedFOpenCall extends FOpenCall { + FOpenCall pair; + + MatchedFOpenCall() { + not pair = this and + pair.getEnclosingFunction() = this.getEnclosingFunction() and + this = followsOpen(pair) + } + + FOpenCall getMatch() { result = pair } +} + +from MatchedFOpenCall fst, FOpenCall snd +where + not isExcluded(fst, IO3Package::fileOpenForReadAndWriteOnDifferentStreamsQuery()) and + // must be opening the same filename + snd = fst.getMatch() and + globalValueNumber(fst.getFilenameExpr()) = globalValueNumber(snd.getFilenameExpr()) and + ( + // different open mode + fst.isReadMode() and snd.isWriteMode() + or + fst.isWriteMode() and snd.isReadMode() + ) +select fst, + "The same file was already opened $@. Files should not be read and written at the same time using different streams.", + snd, "here" diff --git a/c/misra/src/rules/RULE-22-3/standard-example.c b/c/misra/src/rules/RULE-22-3/standard-example.c new file mode 100644 index 000000000..757aa2711 --- /dev/null +++ b/c/misra/src/rules/RULE-22-3/standard-example.c @@ -0,0 +1,5 @@ +#include +void fn(void) { + FILE *fw = fopen("tmp", "r+"); /* "r+" opens for read/write */ + FILE *fr = fopen("tmp", "r"); /* Non-compliant */ +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql b/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql new file mode 100644 index 000000000..c4acbf7ac --- /dev/null +++ b/c/misra/src/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql @@ -0,0 +1,36 @@ +/** + * @id c/misra/attempt-to-write-to-a-read-only-stream + * @name RULE-22-4: There shall be no attempt to write to a stream which has been opened as read-only + * @description Attempting to write on a read-only stream is undefined behavior. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-4 + * correctness + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.standardlibrary.FileAccess +import semmle.code.cpp.dataflow.DataFlow + +class FileDFConf extends DataFlow::Configuration { + FileDFConf() { this = "FileDFConf" } + + override predicate isSource(DataFlow::Node source) { + // source is the return value of a call to fopen + source.asExpr().(FOpenCall).isReadOnlyMode() + } + + override predicate isSink(DataFlow::Node sink) { + // sink must be the second parameter of a FsetposCall call + sink.asExpr() = any(FileWriteFunctionCall write).getFileExpr() + } +} + +from FileDFConf dfConf, DataFlow::Node source, FileWriteFunctionCall sink +where + not isExcluded(sink, IO3Package::attemptToWriteToAReadOnlyStreamQuery()) and + dfConf.hasFlow(source, DataFlow::exprNode(sink.getFileExpr())) +select sink, "Attempt to write to a $@ opened as read-only.", source, "stream" diff --git a/c/misra/src/rules/RULE-22-4/standard-example.c b/c/misra/src/rules/RULE-22-4/standard-example.c new file mode 100644 index 000000000..1b5a2d942 --- /dev/null +++ b/c/misra/src/rules/RULE-22-4/standard-example.c @@ -0,0 +1,6 @@ +#include +void fn(void) { + FILE *fp = fopen("tmp", "r"); + (void)fprintf(fp, "What happens now?"); /* Non-compliant */ + (void)fclose(fp); +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-22-5/PointerToAFileObjectDereferenced.ql b/c/misra/src/rules/RULE-22-5/PointerToAFileObjectDereferenced.ql new file mode 100644 index 000000000..86e0b76e2 --- /dev/null +++ b/c/misra/src/rules/RULE-22-5/PointerToAFileObjectDereferenced.ql @@ -0,0 +1,38 @@ +/** + * @id c/misra/pointer-to-a-file-object-dereferenced + * @name RULE-22-5: A pointer to a FILE object shall not be dereferenced + * @description A FILE object should not be directly manipulated. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-5 + * correctness + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra + +class IndirectlyDereferencedExpr extends Expr { + IndirectlyDereferencedExpr() { + exists(Call call, string names | + names = ["memcmp", "memcpy"] and + ( + call.getTarget().hasGlobalOrStdName(names) + or + exists(MacroInvocation mi | mi.getMacroName() = names and call = mi.getExpr()) + ) and + this = [call.getArgument(0), call.getArgument(1)] + ) + } +} + +from Expr e +where + not isExcluded(e, IO3Package::pointerToAFileObjectDereferencedQuery()) and + ( + e.(PointerDereferenceExpr).getType().hasName("FILE") or + e.(PointerFieldAccess).getQualifier().getType().(DerivedType).getBaseType().hasName("FILE") or + e.(IndirectlyDereferencedExpr).getType().(DerivedType).getBaseType().hasName("FILE") + ) +select e, "Dereferencing an object of type `FILE`." diff --git a/c/misra/src/rules/RULE-22-5/standard-example.c b/c/misra/src/rules/RULE-22-5/standard-example.c new file mode 100644 index 000000000..a9a110ec5 --- /dev/null +++ b/c/misra/src/rules/RULE-22-5/standard-example.c @@ -0,0 +1,8 @@ +#include + +FILE *pf1; +FILE *pf2; +FILE f3; + +pf2 = pf1; /* Compliant */ +f3 = *pf2; /* Non-compliant */ diff --git a/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql b/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql new file mode 100644 index 000000000..457084f35 --- /dev/null +++ b/c/misra/src/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql @@ -0,0 +1,60 @@ +/** + * @id c/misra/eof-shall-be-compared-with-unmodified-return-values + * @name RULE-22-7: The macro EOF shall only be compared with the unmodified return value from any Standard Library + * @description The macro EOF shall only be compared with the unmodified return value from any + * Standard Library function capable of returning EOF + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-7 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.ReadErrorsAndEOF + +/** + * The getchar() return value propagates directly to a check against EOF macro + * type conversions are not allowed + */ +class DFConf extends DataFlow::Configuration { + DFConf() { this = "DFConf" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof InBandErrorReadFunctionCall + } + + override predicate isSink(DataFlow::Node sink) { + exists(EOFWEOFInvocation mi, EqualityOperation eq | + // one operand is the sink + sink.asExpr() = eq.getAnOperand() and + // one operand is an invocation of the EOF macro + mi.getAGeneratedElement() = eq.getAnOperand() + ) + } + + override predicate isBarrier(DataFlow::Node barrier) { + barrier.asExpr() = any(IntegralConversion c).getExpr() + } +} + +// The equality operation `eq` checks a char fetched from `read` against a macro +predicate isWeakMacroCheck(EqualityOperation eq, InBandErrorReadFunctionCall read) { + exists(Expr c, EOFWEOFInvocation mi | + // one operand is the char c fetched from `read` + c = eq.getAnOperand() and + // an operand is an invocation of the EOF macro + mi.getAGeneratedElement() = eq.getAnOperand() and + DataFlow::localExprFlow(read, c) + ) +} + +from EqualityOperation eq, InBandErrorReadFunctionCall read, DFConf dfConf +where + not isExcluded(eq, IO3Package::eofShallBeComparedWithUnmodifiedReturnValuesQuery()) and + isWeakMacroCheck(eq, read) and + not dfConf.hasFlow(DataFlow::exprNode(read), DataFlow::exprNode(eq.getAnOperand())) +select eq, "The check is not reliable as the type of the return value of $@ is converted.", read, + read.toString() diff --git a/c/misra/src/rules/RULE-22-7/standard-example1.c b/c/misra/src/rules/RULE-22-7/standard-example1.c new file mode 100644 index 000000000..5bfcfe840 --- /dev/null +++ b/c/misra/src/rules/RULE-22-7/standard-example1.c @@ -0,0 +1,10 @@ +void f1(void) { + char ch; + ch = (char)getchar(); + /* + * The following test is non-compliant. It will not be reliable as the + * return value is cast to a narrower type before checking for EOF. + */ + if (EOF != (int32_t)ch) { + } +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-22-7/standard-example2.c b/c/misra/src/rules/RULE-22-7/standard-example2.c new file mode 100644 index 000000000..8a0505b8c --- /dev/null +++ b/c/misra/src/rules/RULE-22-7/standard-example2.c @@ -0,0 +1,19 @@ +void f2(void) { + char ch; + ch = (char)getchar(); + if (!feof(stdin)) { + } +} + +void f3(void) { + int32_t i_ch; + i_ch = getchar(); + /* + * The following test is compliant. It will be reliable as the + * unconverted return value is used when checking for EOF. + */ + if (EOF != i_ch) { + char ch; + ch = (char)i_ch; + } +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.ql b/c/misra/src/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.ql new file mode 100644 index 000000000..deea2afa8 --- /dev/null +++ b/c/misra/src/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.ql @@ -0,0 +1,24 @@ +/** + * @id c/misra/precaution-include-guards-not-provided + * @name RULE-4-10: Precautions shall be taken in order to prevent the contents of a header file being included more than once + * @description Using anything other than a standard include guard form can make code confusing and + * can lead to multiple or conflicting definitions. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-4-10 + * correctness + * maintainability + * readability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.includeguardsnotused.IncludeGuardsNotUsed + +class PrecautionIncludeGuardsNotProvidedQuery extends IncludeGuardsNotUsedSharedQuery { + PrecautionIncludeGuardsNotProvidedQuery() { + this = Preprocessor2Package::precautionIncludeGuardsNotProvidedQuery() + } +} diff --git a/c/misra/src/rules/RULE-4-10/standard-example.c b/c/misra/src/rules/RULE-4-10/standard-example.c new file mode 100644 index 000000000..65b8a2685 --- /dev/null +++ b/c/misra/src/rules/RULE-4-10/standard-example.c @@ -0,0 +1,12 @@ + +#if !defined ( identifier ) +#define identifier +/* Contents of file */ +#endif + + +#ifndef identifier +#define identifier +/* Contents of file */ +#endif + \ No newline at end of file diff --git a/c/misra/test/qlpack.yml b/c/misra/test/qlpack.yml index 87c89cb5c..2e9d46e7b 100644 --- a/c/misra/test/qlpack.yml +++ b/c/misra/test/qlpack.yml @@ -1,4 +1,4 @@ name: misra-c-coding-standards-tests -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: misra-c-coding-standards extractor: cpp \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.expected b/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.expected new file mode 100644 index 000000000..406010428 --- /dev/null +++ b/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.expected @@ -0,0 +1 @@ +| test.c:25:1:25:29 | #define MACROTHIRTEEN(X) #X ## X | Macro definition uses an # operator followed by a ## operator. | diff --git a/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.qlref b/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.qlref new file mode 100644 index 000000000..35ef457ca --- /dev/null +++ b/c/misra/test/rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.qlref @@ -0,0 +1 @@ +rules/RULE-20-11/MoreThanOneHashOperatorInMacroDefinition.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-11/test.c b/c/misra/test/rules/RULE-20-11/test.c new file mode 100644 index 000000000..ad2c20597 --- /dev/null +++ b/c/misra/test/rules/RULE-20-11/test.c @@ -0,0 +1,27 @@ +#define MACROONE 1 // COMPLIANT + +#define MACROTWO '#\'-#' + '#' // COMPLIANT + +#define MACROTHREE "##" // COMPLIANT + +#define MACROFOUR "##" + "#" // COMPLIANT + +#define MACROFIVE(X) #X // COMPLIANT + +#define MACROSIX(X, Y) X##Y // COMPLIANT + +#define MACROSEVEN "##'" #"#" // COMPLIANT + +#define MACROEIGHT '##' #"#" // COMPLIANT + +#define MACRONINE "##\"\"" + "#" // COMPLIANT + +#define MACROTEN "##\"\"'" + "#" // COMPLIANT + +#define MACROELEVEN(X) X #X #X // COMPLIANT + +#define MACROTWELVE(X) X##X##X // COMPLIANT + +#define MACROTHIRTEEN(X) #X##X // NON_COMPLIANT + +#define MACROFOURTEEN '#\'-#' + 1 #1 #1 + '#' // COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.expected b/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.expected new file mode 100644 index 000000000..be347218b --- /dev/null +++ b/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.expected @@ -0,0 +1,2 @@ +| test.c:4:1:4:41 | #define BAD_MACRO_WITH_ARG(x) (x) + wow ## x | Macro BAD_MACRO_WITH_ARG contains use of parameter x used in multiple contexts. | +| test.c:5:1:5:48 | #define BAD_MACRO_WITH_ARG_TWO(x,y) (x) + wow ## x | Macro BAD_MACRO_WITH_ARG_TWO contains use of parameter x used in multiple contexts. | diff --git a/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.qlref b/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.qlref new file mode 100644 index 000000000..a2edc3acc --- /dev/null +++ b/c/misra/test/rules/RULE-20-12/MacroParameterUsedAsHashOperand.qlref @@ -0,0 +1 @@ +rules/RULE-20-12/MacroParameterUsedAsHashOperand.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-12/test.c b/c/misra/test/rules/RULE-20-12/test.c new file mode 100644 index 000000000..768238f36 --- /dev/null +++ b/c/misra/test/rules/RULE-20-12/test.c @@ -0,0 +1,25 @@ + +#define GOOD_MACRO_WITH_ARG(X) ((X)*X##_scale) // COMPLIANT +#define MACRO 1 +#define BAD_MACRO_WITH_ARG(x) (x) + wow##x // NON_COMPLIANT +#define BAD_MACRO_WITH_ARG_TWO(x, y) (x) + wow##x // NON_COMPLIANT +#define MACROONE(x) #x // COMPLIANT +#define MACROTWO(x) x *x // COMPLIANT +#define MACROTHREE(x) "##\"\"'" + (x) // COMPLIANT +#define FOO(x) #x MACROONE(x) // COMPLIANT - no further arg expansion + +void f() { + + int x; + int x_scale; + int y; + int wowMACRO = 0; + + y = GOOD_MACRO_WITH_ARG(x); + wowMACRO = BAD_MACRO_WITH_ARG(MACRO); + wowMACRO = BAD_MACRO_WITH_ARG_TWO(MACRO, 1); + char s[] = MACROONE(MACRO); + y = MACROTWO(MACRO); + MACROTHREE(MACRO); + FOO(x); +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.expected b/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.expected new file mode 100644 index 000000000..0cda5466c --- /dev/null +++ b/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.expected @@ -0,0 +1 @@ +| test.c:2:1:2:11 | #undef TEST | Use of undef found. | diff --git a/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.qlref b/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.qlref new file mode 100644 index 000000000..632afdd4d --- /dev/null +++ b/c/misra/test/rules/RULE-20-5/UndefShouldNotBeUsed.qlref @@ -0,0 +1 @@ +rules/RULE-20-5/UndefShouldNotBeUsed.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-20-5/test.c b/c/misra/test/rules/RULE-20-5/test.c new file mode 100644 index 000000000..d9b96e586 --- /dev/null +++ b/c/misra/test/rules/RULE-20-5/test.c @@ -0,0 +1,2 @@ +#define TEST 1 // COMPLIANT +#undef TEST // NON_COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.expected b/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.expected new file mode 100644 index 000000000..6111072ba --- /dev/null +++ b/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.expected @@ -0,0 +1,5 @@ +| test.c:6:14:6:18 | call to fopen | The same file was already opened $@. Files should not be read and written at the same time using different streams. | test.c:5:14:5:18 | call to fopen | here | +| test.c:17:14:17:18 | call to fopen | The same file was already opened $@. Files should not be read and written at the same time using different streams. | test.c:16:14:16:18 | call to fopen | here | +| test.c:33:14:33:18 | call to fopen | The same file was already opened $@. Files should not be read and written at the same time using different streams. | test.c:32:14:32:18 | call to fopen | here | +| test.c:43:14:43:18 | call to fopen | The same file was already opened $@. Files should not be read and written at the same time using different streams. | test.c:42:14:42:18 | call to fopen | here | +| test.c:59:14:59:18 | call to fopen | The same file was already opened $@. Files should not be read and written at the same time using different streams. | test.c:58:14:58:18 | call to fopen | here | diff --git a/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.qlref b/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.qlref new file mode 100644 index 000000000..6a67ba596 --- /dev/null +++ b/c/misra/test/rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.qlref @@ -0,0 +1 @@ +rules/RULE-22-3/FileOpenForReadAndWriteOnDifferentStreams.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-3/test.c b/c/misra/test/rules/RULE-22-3/test.c new file mode 100644 index 000000000..7e51ee996 --- /dev/null +++ b/c/misra/test/rules/RULE-22-3/test.c @@ -0,0 +1,60 @@ +#include +#include + +void f1(void) { + FILE *fw = fopen("tmp1", "r+"); + FILE *fr = fopen("tmp1", "r"); // NON_COMPLIANT +} + +void f2(void) { + FILE *fw = fopen("tmp2", "r+"); + fclose(fw); + FILE *fr = fopen("tmp2", "r"); // COMPLIANT +} + +void f3(void) { + FILE *fr = fopen("tmp3", "r"); + FILE *fw = fopen("tmp3", "r+"); // NON_COMPLIANT + fclose(fw); +} + +void f4(void) { + FILE *fw = fopen("tmp4", "r"); + FILE *fr = fopen("tmp4", "r"); // COMPLIANT +} + +void f5(void) { + FILE *fr = fopen("tmp5a", "r"); + FILE *fw = fopen("tmp5b", "r+"); // COMPLIANT +} + +void f6(void) { + FILE *fw = fopen("tmp6", "w"); + FILE *fr = fopen("tmp6", "r"); // NON_COMPLIANT +} + +void f7(void) { + FILE *fw = fopen("tmp1", "r"); // COMPLIANT +} + +void f8(void) { + char file[] = "tmp8"; + FILE *fw = fopen(file, "r+"); + FILE *fr = fopen(file, "r"); // NON_COMPLIANT +} + +void f9(void) { + char name[50] = "tmp9"; + char ext[] = "txt"; + char file[] = strcat(name, ext); + FILE *fw = fopen(file, "r+"); + FILE *fr = fopen(strcat(name, ext), "r"); // NON_COMPLIANT[FALSE_NEGATIVE] +} + +void f10(void) { + char name[50] = "tmp10"; + char ext[] = "txt"; + strcat(name, ext); + FILE *fw = fopen(name, "r+"); + FILE *fr = fopen(name, "r"); // NON_COMPLIANT +} diff --git a/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.expected b/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.expected new file mode 100644 index 000000000..0bfce133c --- /dev/null +++ b/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.expected @@ -0,0 +1,2 @@ +| test.c:10:3:10:9 | call to fprintf | Attempt to write to a $@ opened as read-only. | test.c:9:14:9:18 | call to fopen | stream | +| test.c:15:3:15:9 | call to fprintf | Attempt to write to a $@ opened as read-only. | test.c:18:14:18:18 | call to fopen | stream | diff --git a/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.qlref b/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.qlref new file mode 100644 index 000000000..c636f2d9a --- /dev/null +++ b/c/misra/test/rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.qlref @@ -0,0 +1 @@ +rules/RULE-22-4/AttemptToWriteToAReadOnlyStream.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-4/test.c b/c/misra/test/rules/RULE-22-4/test.c new file mode 100644 index 000000000..c69e2fd4f --- /dev/null +++ b/c/misra/test/rules/RULE-22-4/test.c @@ -0,0 +1,21 @@ +#include +void f0() { + FILE *fp = fopen("file", "r+"); + fprintf(fp, "text"); // COMPLIANT + fclose(fp); +} + +void f1() { + FILE *fp = fopen("file", "r"); + fprintf(fp, "text"); // NON_COMPLIANT + fclose(fp); +} + +void f2help(FILE *fp) { + fprintf(fp, "text"); // NON_COMPLIANT +} +void f2() { + FILE *fp = fopen("file", "r"); + f2help(fp); + fclose(fp); +} diff --git a/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.expected b/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.expected new file mode 100644 index 000000000..81c499eaf --- /dev/null +++ b/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.expected @@ -0,0 +1,7 @@ +| test.c:13:8:13:11 | * ... | Dereferencing an object of type `FILE`. | +| test.c:14:8:14:10 | pos | Dereferencing an object of type `FILE`. | +| test.c:16:10:16:12 | pf1 | Dereferencing an object of type `FILE`. | +| test.c:16:15:16:17 | pf2 | Dereferencing an object of type `FILE`. | +| test.c:17:10:17:12 | pf1 | Dereferencing an object of type `FILE`. | +| test.c:17:15:17:17 | pf2 | Dereferencing an object of type `FILE`. | +| test.c:22:8:22:10 | pos | Dereferencing an object of type `FILE`. | diff --git a/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.qlref b/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.qlref new file mode 100644 index 000000000..a35c11f4d --- /dev/null +++ b/c/misra/test/rules/RULE-22-5/PointerToAFileObjectDereferenced.qlref @@ -0,0 +1 @@ +rules/RULE-22-5/PointerToAFileObjectDereferenced.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-5/test.c b/c/misra/test/rules/RULE-22-5/test.c new file mode 100644 index 000000000..bd1463b80 --- /dev/null +++ b/c/misra/test/rules/RULE-22-5/test.c @@ -0,0 +1,28 @@ +//#include +#include + +typedef struct { + int pos; +} FILE; + +void f() { + FILE *pf1; + FILE *pf2; + FILE f3; + pf2 = pf1; // COMPLIANT + f3 = *pf2; // NON_COMPLIANT + pf1->pos = 0; // NON_COMPLIANT + + memcpy(pf1, pf2, 1); // NON_COMPLIANT + memcmp(pf1, pf2, 1); // NON_COMPLIANT +} + +void f1help(FILE *pf1, FILE *pf2) { + pf2 = pf1; // COMPLIANT + pf1->pos = 0; // NON_COMPLIANT +} +void f1() { + FILE *pf1; + FILE *pf2; + f1help(pf1, pf2); +} diff --git a/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.expected b/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.expected new file mode 100644 index 000000000..709d8b002 --- /dev/null +++ b/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.expected @@ -0,0 +1,2 @@ +| test.c:6:7:6:20 | ... != ... | The check is not reliable as the type of the return value of $@ is converted. | test.c:5:14:5:20 | call to getchar | call to getchar | +| test.c:13:7:13:15 | ... != ... | The check is not reliable as the type of the return value of $@ is converted. | test.c:12:14:12:20 | call to getchar | call to getchar | diff --git a/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.qlref b/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.qlref new file mode 100644 index 000000000..78887cb47 --- /dev/null +++ b/c/misra/test/rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.qlref @@ -0,0 +1 @@ +rules/RULE-22-7/EofShallBeComparedWithUnmodifiedReturnValues.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-7/test.c b/c/misra/test/rules/RULE-22-7/test.c new file mode 100644 index 000000000..810096951 --- /dev/null +++ b/c/misra/test/rules/RULE-22-7/test.c @@ -0,0 +1,31 @@ +#include + +void f1a(void) { + char ch; + ch = (char)getchar(); + if (EOF != (int)ch) { // NON_COMPLIANT + } +} + +void f1b(void) { + int ch; + ch = (char)getchar(); + if (EOF != ch) { // NON_COMPLIANT + } +} + +void f2(void) { + char ch; + ch = (char)getchar(); + if (!feof(stdin)) { // COMPLIANT + } +} + +void f3(void) { + int i_ch; + i_ch = getchar(); + if (EOF != i_ch) { // COMPLIANT + char ch; + ch = (char)i_ch; + } +} diff --git a/c/misra/test/rules/RULE-4-10/NonUniqueIncludeGuards.testref b/c/misra/test/rules/RULE-4-10/NonUniqueIncludeGuards.testref new file mode 100644 index 000000000..e38907a2f --- /dev/null +++ b/c/misra/test/rules/RULE-4-10/NonUniqueIncludeGuards.testref @@ -0,0 +1 @@ +c/common/test/rules/nonuniqueincludeguardsused/NonUniqueIncludeGuardsUsed.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.testref b/c/misra/test/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.testref new file mode 100644 index 000000000..065354082 --- /dev/null +++ b/c/misra/test/rules/RULE-4-10/PrecautionIncludeGuardsNotProvided.testref @@ -0,0 +1 @@ +c/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql \ No newline at end of file diff --git a/change_notes/2022-05-20-M16-3-1-exclude-non-function-macros.md b/change_notes/2022-05-20-M16-3-1-exclude-non-function-macros.md new file mode 100644 index 000000000..e3ff564ca --- /dev/null +++ b/change_notes/2022-05-20-M16-3-1-exclude-non-function-macros.md @@ -0,0 +1,2 @@ +- `M16-3-1` - `MoreThanOneOccurrenceHashOperatorInMacroDefinition.ql`: + - Removes detection of more than one occurrence in non function like Macros. \ No newline at end of file diff --git a/cpp/autosar/src/qlpack.yml b/cpp/autosar/src/qlpack.yml index 31eb8beee..2d991b16e 100644 --- a/cpp/autosar/src/qlpack.yml +++ b/cpp/autosar/src/qlpack.yml @@ -1,4 +1,4 @@ name: autosar-cpp-coding-standards -version: 2.3.0 +version: 2.4.0 suites: codeql-suites libraryPathDependencies: common-cpp-coding-standards diff --git a/cpp/autosar/src/rules/M16-2-3/IncludeGuardsNotProvided.ql b/cpp/autosar/src/rules/M16-2-3/IncludeGuardsNotProvided.ql index 9dd06713d..5cc518f8f 100644 --- a/cpp/autosar/src/rules/M16-2-3/IncludeGuardsNotProvided.ql +++ b/cpp/autosar/src/rules/M16-2-3/IncludeGuardsNotProvided.ql @@ -17,54 +17,10 @@ import cpp import codingstandards.cpp.autosar +import codingstandards.cpp.rules.includeguardsnotused.IncludeGuardsNotUsed -pragma[noinline] -predicate isPreprocFileAndLine(Element pd, string filepath, int startLine) { - pd.getLocation().hasLocationInfo(filepath, startLine, _, _, _) -} - -pragma[noinline] -predicate isPreprocConditionalRange( - PreprocessorBranch pb, string filepath, int startLine, int endLine -) { - exists(PreprocessorEndif end | pb.getEndIf() = end | - isPreprocFileAndLine(pb, filepath, startLine) and - isPreprocFileAndLine(end, filepath, endLine) - ) -} - -class GuardMacro extends Macro { - GuardMacro() { - //wrapper #ifndef present for use as include guard - //and they only suffice if there are no non comment elements above them - exists(PreprocessorIfndef wrapper | - getAGuard(this) = wrapper and - not exists(Element above, string filepath, int aboveStartLine, int ifdefStartLine | - aboveStartLine < ifdefStartLine and - isPreprocFileAndLine(wrapper, filepath, ifdefStartLine) and - isPreprocFileAndLine(above, filepath, aboveStartLine) and - not (above instanceof Comment or above instanceof File) - ) - ) +class PrecautionIncludeGuardsNotProvidedQuery extends IncludeGuardsNotUsedSharedQuery { + PrecautionIncludeGuardsNotProvidedQuery() { + this = IncludesPackage::includeGuardsNotProvidedQuery() } } - -/** - * An optimised version of `PreprocessorBranchDirective.getAGuard()`. - */ -private PreprocessorBranch getAGuard(Element guardedElement) { - exists(string filepath, int ifStartLine, int guardedElementStartLine, int endifStartLine | - isPreprocConditionalRange(result, filepath, ifStartLine, endifStartLine) and - isPreprocFileAndLine(guardedElement, filepath, guardedElementStartLine) and - ifStartLine < guardedElementStartLine and - guardedElementStartLine < endifStartLine - ) -} - -from File file -where - not exists(GuardMacro guard | guard.getFile() = file) and - //headers are anything included - exists(Include i | i.getIncludedFile() = file) and - not isExcluded(file, IncludesPackage::includeGuardsNotProvidedQuery()) -select file, "Header file $@ is missing expected include guard.", file, file.getBaseName() diff --git a/cpp/autosar/src/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.ql b/cpp/autosar/src/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.ql index 5c6243a7d..96fd46907 100644 --- a/cpp/autosar/src/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.ql +++ b/cpp/autosar/src/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.ql @@ -15,16 +15,17 @@ import cpp import codingstandards.cpp.autosar +import codingstandards.cpp.Macro -from Macro m, string body +from Macro m where - body = - m.getBody() - .regexpReplaceAll("#+", "#") - .regexpReplaceAll("\\\\\"", "") - .regexpReplaceAll("\\\\'", "") - .regexpReplaceAll("\"[^\"]+\"", "") - .regexpReplaceAll("'[^']+'", "") and - exists(int c | c = count(int x | x = body.indexOf("#")) and c > 1) and + count(StringizingOperator op | op.getMacro() = m | op) > 1 + or + count(any(TokenPastingOperator op | op.getMacro() = m | op.getOffset())) > 1 + or + exists(StringizingOperator one, TokenPastingOperator two | + one.getMacro() = m and + two.getMacro() = m + ) and not isExcluded(m, MacrosPackage::moreThanOneOccurrenceHashOperatorInMacroDefinitionQuery()) select m, "Macro definition uses the # or ## operator more than once." diff --git a/cpp/autosar/test/qlpack.yml b/cpp/autosar/test/qlpack.yml index 195698fe8..1c37efaa1 100644 --- a/cpp/autosar/test/qlpack.yml +++ b/cpp/autosar/test/qlpack.yml @@ -1,4 +1,4 @@ name: autosar-cpp-coding-standards-tests -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: autosar-cpp-coding-standards extractor: cpp diff --git a/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.expected b/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.expected deleted file mode 100644 index 9d17b9fc4..000000000 --- a/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.expected +++ /dev/null @@ -1,3 +0,0 @@ -| headers/test3.hpp:0:0:0:0 | headers/test3.hpp | Header file $@ is missing expected include guard. | headers/test3.hpp:0:0:0:0 | headers/test3.hpp | test3.hpp | -| headers/test4.hpp:0:0:0:0 | headers/test4.hpp | Header file $@ is missing expected include guard. | headers/test4.hpp:0:0:0:0 | headers/test4.hpp | test4.hpp | -| headers/test5.hpp:0:0:0:0 | headers/test5.hpp | Header file $@ is missing expected include guard. | headers/test5.hpp:0:0:0:0 | headers/test5.hpp | test5.hpp | diff --git a/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.qlref b/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.qlref deleted file mode 100644 index bc180f825..000000000 --- a/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/M16-2-3/IncludeGuardsNotProvided.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.testref b/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.testref new file mode 100644 index 000000000..2fac3f74c --- /dev/null +++ b/cpp/autosar/test/rules/M16-2-3/IncludeGuardsNotProvided.testref @@ -0,0 +1 @@ +cpp/common/test/rules/includeguardsnotused/IncludeGuardsNotUsed.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M16-2-3/NonUniqueIncludeGuardsCpp.testref b/cpp/autosar/test/rules/M16-2-3/NonUniqueIncludeGuardsCpp.testref new file mode 100644 index 000000000..e8adc6dc8 --- /dev/null +++ b/cpp/autosar/test/rules/M16-2-3/NonUniqueIncludeGuardsCpp.testref @@ -0,0 +1 @@ +cpp/common/test/rules/nonuniqueincludeguardsused/NonUniqueIncludeGuardsUsed.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.expected b/cpp/autosar/test/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.expected index e957365b3..96c5d3449 100644 --- a/cpp/autosar/test/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.expected +++ b/cpp/autosar/test/rules/M16-3-1/MoreThanOneOccurrenceHashOperatorInMacroDefinition.expected @@ -1,4 +1,3 @@ | test.cpp:21:1:21:29 | #define MACROELEVEN(X) X #X #X | Macro definition uses the # or ## operator more than once. | | test.cpp:23:1:23:29 | #define MACROTWELVE(X) X ## X ## X | Macro definition uses the # or ## operator more than once. | | test.cpp:25:1:25:29 | #define MACROTHIRTEEN(X) #X ## X | Macro definition uses the # or ## operator more than once. | -| test.cpp:27:1:27:45 | #define MACROFOURTEEN '#\\'-#' + 1 #1 #1 + '#' | Macro definition uses the # or ## operator more than once. | diff --git a/cpp/autosar/test/rules/M16-3-1/test.cpp b/cpp/autosar/test/rules/M16-3-1/test.cpp index df1cf8b8a..e8b7f752f 100644 --- a/cpp/autosar/test/rules/M16-3-1/test.cpp +++ b/cpp/autosar/test/rules/M16-3-1/test.cpp @@ -24,4 +24,5 @@ #define MACROTHIRTEEN(X) #X##X // NON_COMPLIANT -#define MACROFOURTEEN '#\'-#' + 1 #1 #1 + '#' // NON_COMPLIANT \ No newline at end of file +#define MACROFOURTEEN \ + '#\'-#' + 1 #1 #1 + '#' // COMPLIANT, not a function-like macro. \ No newline at end of file diff --git a/cpp/cert/src/qlpack.yml b/cpp/cert/src/qlpack.yml index 003fb7b7c..b7f113b39 100644 --- a/cpp/cert/src/qlpack.yml +++ b/cpp/cert/src/qlpack.yml @@ -1,4 +1,4 @@ name: cert-cpp-coding-standards -version: 2.3.0 +version: 2.4.0 suites: codeql-suites libraryPathDependencies: common-cpp-coding-standards diff --git a/cpp/cert/test/qlpack.yml b/cpp/cert/test/qlpack.yml index 11d4756c2..f212d1000 100644 --- a/cpp/cert/test/qlpack.yml +++ b/cpp/cert/test/qlpack.yml @@ -1,4 +1,4 @@ name: cert-cpp-coding-standards-tests -version: 2.3.0 +version: 2.4.0 libraryPathDependencies: cert-cpp-coding-standards extractor: cpp diff --git a/cpp/common/src/codingstandards/cpp/CharFunctions.qll b/cpp/common/src/codingstandards/cpp/CharFunctions.qll new file mode 100644 index 000000000..352f61858 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/CharFunctions.qll @@ -0,0 +1,31 @@ +import cpp + +/** + * Models a class of functions that are either testers of characters + * or standard library conversion functions. + */ +class CToOrIsCharFunction extends Function { + CToOrIsCharFunction() { + this instanceof CIsCharFunction or + this instanceof CToCharFunction + } +} + +/** + * Models a class of functions that test characters. + */ +class CIsCharFunction extends Function { + CIsCharFunction() { + getName() in [ + "isalnum", "isalpha", "isascii", "isblank", "iscntrl", "isdigit", "isgraph", "islower", + "isprint", "ispunct", "isspace", "isupper", "isxdigit", "__isspace" + ] + } +} + +/** + * Models a class of functions convert characters. + */ +class CToCharFunction extends Function { + CToCharFunction() { getName() in ["toascii", "toupper", "tolower"] } +} diff --git a/cpp/common/src/codingstandards/cpp/Dereferenced.qll b/cpp/common/src/codingstandards/cpp/Dereferenced.qll index f2fde03f3..ffcd74728 100644 --- a/cpp/common/src/codingstandards/cpp/Dereferenced.qll +++ b/cpp/common/src/codingstandards/cpp/Dereferenced.qll @@ -24,6 +24,9 @@ class DereferencedExpr extends Expr { c.getTarget() instanceof StarOperator ) or + // And access to arrays `array[0]` + this = any(ArrayExpr ae).getArrayBase() + or this instanceof StandardLibraryDereferencedExpr ) } diff --git a/cpp/common/src/codingstandards/cpp/FgetsErrorManagement.qll b/cpp/common/src/codingstandards/cpp/FgetsErrorManagement.qll index 9ca59bc4c..4f99b02e2 100644 --- a/cpp/common/src/codingstandards/cpp/FgetsErrorManagement.qll +++ b/cpp/common/src/codingstandards/cpp/FgetsErrorManagement.qll @@ -49,6 +49,7 @@ class FgetsLikeCall extends FunctionCall { class BooleanFgetsExpr extends Expr { boolean isNull; + boolean isNotNull; FgetsLikeCall fgetCall; Expr operand; @@ -56,6 +57,7 @@ class BooleanFgetsExpr extends Expr { // if(fgets) fgetCall = this and isNull = false and + isNotNull = true and operand = this or exists(BooleanFgetsExpr e | @@ -63,50 +65,47 @@ class BooleanFgetsExpr extends Expr { operand = e and fgetCall = e.getFgetCall() and ( - // if(x==0) - e = this.(EQExpr).getAnOperand() and - this.(EQExpr).getAnOperand() instanceof NullValue and - isNull = e.isNull().booleanNot() - or - // if(x!=0) - e = this.(NEExpr).getAnOperand() and - this.(NEExpr).getAnOperand() instanceof NullValue and - isNull = e.isNull() - or - // if(x) - DataFlow::localExprFlow(e, this) and - isNull = e.isNull() - or - // if(!x) - e = this.(NotExpr).getOperand() and - isNull = e.isNull().booleanNot() - or - // if(x && cond) - e = this.(LogicalAndExpr).getAnOperand() and + isNotNull = isNull.booleanNot() and ( - e.isNull() = false and - isNull = false + // if(e==0) + e = this.(EQExpr).getAnOperand() and + this.(EQExpr).getAnOperand() instanceof NullValue and + isNull = e.isNull().booleanNot() + or + // if(e!=0) + e = this.(NEExpr).getAnOperand() and + this.(NEExpr).getAnOperand() instanceof NullValue and + isNull = e.isNull() or - e.isNull() = true and - ( - isNull = true - or - isNull = false - ) + // if(e) + DataFlow::localExprFlow(e, this) and + isNull = e.isNull() + or + // if(!e) + e = this.(NotExpr).getOperand() and + isNull = e.isNull().booleanNot() + or + // if(cond && e) + e = this.(LogicalAndExpr).getRightOperand() and + isNull = e.isNull() + or + // if(cond || e) + e = this.(LogicalOrExpr).getRightOperand() and + isNull = e.isNull() ) or - // if(x || cond) - e = this.(LogicalOrExpr).getAnOperand() and + // if(e && cond) + e = this.(LogicalAndExpr).getLeftOperand() and ( - e.isNull() = true and - isNull = true - or - e.isNull() = false and - ( - isNull = true - or - isNull = false - ) + isNull = false and + isNotNull = false + ) + or + // if(e || cond) + e = this.(LogicalOrExpr).getLeftOperand() and + ( + isNull = true and + isNotNull = true ) ) ) @@ -114,14 +113,22 @@ class BooleanFgetsExpr extends Expr { boolean isNull() { result = isNull } + boolean isNotNull() { result = isNotNull } + Expr getFgetCall() { result = fgetCall } Expr getOperand() { result = operand } } +/* + * A guard controlled by a `BooleanFgetsExpr` + */ + class FgetsGuard extends BooleanFgetsExpr { FgetsGuard() { - exists(IfStmt i | i.getCondition() = this) or exists(Loop i | i.getCondition() = this) + exists(IfStmt i | i.getCondition() = this) + or + exists(Loop i | i.getCondition() = this) } Stmt getThenSuccessor() { @@ -153,8 +160,10 @@ class FgetsGuard extends BooleanFgetsExpr { } ControlFlowNode getNonNullSuccessor() { - this.isNull() = true and result = this.getElseSuccessor() + this.isNotNull() = true and + result = this.getThenSuccessor() or - this.isNull() = false and result = getThenSuccessor() + this.isNotNull() = false and + result = getElseSuccessor() } } diff --git a/cpp/common/src/codingstandards/cpp/Macro.qll b/cpp/common/src/codingstandards/cpp/Macro.qll new file mode 100644 index 000000000..24b0bc437 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/Macro.qll @@ -0,0 +1,65 @@ +import cpp + +/** + * Macros with a parameter + */ +class FunctionLikeMacro extends Macro { + FunctionLikeMacro() { this.getHead().regexpMatch("[_a-zA-Z0-9]+\\s*\\([^\\)]*?\\)") } + + string getParameter(int i) { + result = + this.getHead().regexpCapture("[_a-zA-Z0-9]+\\s*\\(([^\\)]*)\\)", 1).splitAt(",", i).trim() + } + + string getAParameter() { result = getParameter(_) } + + int getAParameterUse(int index) { + exists(string parameter | parameter = getParameter(index) | + result = this.getBody().indexOf(parameter) + ) + } +} + +newtype TMacroOperator = + TTokenPastingOperator(FunctionLikeMacro m, string operand, int operatorOffset, int operandOffset) { + m.getAParameter() = operand and + ( + exists(string match | + match = m.getBody().regexpFind("#{2}\\s*" + operand, _, operatorOffset) + | + operandOffset = operatorOffset + match.indexOf(operand) + ) + or + exists(string match | match = m.getBody().regexpFind(operand + "\\s*#{2}", _, operandOffset) | + operatorOffset = operandOffset + match.indexOf("##") + ) + ) + } or + TStringizingOperator(FunctionLikeMacro m, string operand, int operatorOffset, int operandOffset) { + operand = m.getAParameter() and + exists(string match | + match = m.getBody().regexpFind("(? or ""filename"" sequence",,Preprocessor,Easy, +c,MISRA-C-2012,RULE-20-3,No,Required,,,"The #include directive shall be followed by either a or ""filename"" sequence",,,Easy,This is verified by the compiler c,MISRA-C-2012,RULE-20-4,Yes,Required,,,A macro shall not be defined with the same name as a keyword,A17-0-1,Preprocessor,Medium, -c,MISRA-C-2012,RULE-20-5,Yes,Advisory,,,#undef should not be used,,Preprocessor,Easy, +c,MISRA-C-2012,RULE-20-5,Yes,Advisory,,,#undef should not be used,,Preprocessor2,Easy, c,MISRA-C-2012,RULE-20-6,Yes,Required,,,Tokens that look like a preprocessing directive shall not occur within a macro argument,M16-0-5,Preprocessor,Import, c,MISRA-C-2012,RULE-20-7,Yes,Required,,,Expressions resulting from the expansion of macro parameters shall be enclosed in parentheses,M16-0-6,Preprocessor,Easy, -c,MISRA-C-2012,RULE-20-8,Yes,Required,,,The controlling expression of a #if or #elif preprocessing directive shall evaluate to 0 or 1,,Preprocessor,Hard, +c,MISRA-C-2012,RULE-20-8,Yes,Required,,,The controlling expression of a #if or #elif preprocessing directive shall evaluate to 0 or 1,,,Hard, c,MISRA-C-2012,RULE-20-9,Yes,Required,,,All identifiers used in the controlling expression of #if or #elif preprocessing directives shall be #defined before evaluation,M16-0-7,Preprocessor1,Import, c,MISRA-C-2012,RULE-20-10,Yes,Advisory,,,The # and ## preprocessor operators should not be used,M16-3-2,Preprocessor1,Import, -c,MISRA-C-2012,RULE-20-11,Yes,Required,,,A macro parameter immediately following a # operator shall not immediately be followed by a ## operator,M16-3-1,Preprocessor,Easy, -c,MISRA-C-2012,RULE-20-12,Yes,Required,,,"A macro parameter used as an operand to the # or ## operators, which is itself subject to further macro replacement, shall only be used as an operand to these operators",,Preprocessor,Medium, -c,MISRA-C-2012,RULE-20-13,Yes,Required,,,A line whose first token is # shall be a valid preprocessing directive,M16-0-8,Preprocessor,Easy, -c,MISRA-C-2012,RULE-20-14,Yes,Required,,,"All #else, #elif and #endif preprocessor directives shall reside in the same file as the #if, #ifdef or #ifndef directive to which they are related",M16-1-2,Preprocessor,Import, -c,MISRA-C-2012,RULE-21-1,Yes,Required,,,#define and #undef shall not be used on a reserved identifier or reserved macro name,,Preprocessor,Hard, +c,MISRA-C-2012,RULE-20-11,Yes,Required,,,A macro parameter immediately following a # operator shall not immediately be followed by a ## operator,M16-3-1,Preprocessor2,Easy, +c,MISRA-C-2012,RULE-20-12,Yes,Required,,,"A macro parameter used as an operand to the # or ## operators, which is itself subject to further macro replacement, shall only be used as an operand to these operators",,Preprocessor2,Medium, +c,MISRA-C-2012,RULE-20-13,No,Required,,,A line whose first token is # shall be a valid preprocessing directive,M16-0-8,,,This is verified by the compiler in the cases where the token is not within an ifdef branch that's never taken +c,MISRA-C-2012,RULE-20-14,No,Required,,,"All #else, #elif and #endif preprocessor directives shall reside in the same file as the #if, #ifdef or #ifndef directive to which they are related",M16-1-2,,,Compilers already prohibit this case +c,MISRA-C-2012,RULE-21-1,Yes,Required,,,#define and #undef shall not be used on a reserved identifier or reserved macro name,,,Hard, c,MISRA-C-2012,RULE-21-2,Yes,Required,,,A reserved identifier or reserved macro name shall not be declared,,Declarations,Hard, c,MISRA-C-2012,RULE-21-3,Yes,Required,,,The memory allocation and deallocation functions of shall not be used,,Banned,Medium, c,MISRA-C-2012,RULE-21-4,Yes,Required,,,The standard header file shall not be used ,,Banned,Easy, @@ -766,11 +766,11 @@ c,MISRA-C-2012,RULE-21-20,Yes,Mandatory,,,"The pointer returned by the Standard c,MISRA-C-2012,RULE-21-21,Yes,Required,,,The Standard Library function system of shall not be used,ENV33-C,Banned,Import, c,MISRA-C-2012,RULE-22-1,Yes,Required,,,All resources obtained dynamically by means of Standard Library functions shall be explicitly released,,Memory,Hard, c,MISRA-C-2012,RULE-22-2,Yes,Mandatory,,,A block of memory shall only be freed if it was allocated by means of a Standard Library function,,Memory,Hard, -c,MISRA-C-2012,RULE-22-3,Yes,Required,,,The same file shall not be open for read and write access at the same time on different streams,,IO,Hard, -c,MISRA-C-2012,RULE-22-4,Yes,Mandatory,,,There shall be no attempt to write to a stream which has been opened as read-only,,IO,Medium, -c,MISRA-C-2012,RULE-22-5,Yes,Mandatory,,,A pointer to a FILE object shall not be dereferenced,,IO,Medium, +c,MISRA-C-2012,RULE-22-3,Yes,Required,,,The same file shall not be open for read and write access at the same time on different streams,,IO3,Hard, +c,MISRA-C-2012,RULE-22-4,Yes,Mandatory,,,There shall be no attempt to write to a stream which has been opened as read-only,,IO3,Medium, +c,MISRA-C-2012,RULE-22-5,Yes,Mandatory,,,A pointer to a FILE object shall not be dereferenced,,IO3,Medium, c,MISRA-C-2012,RULE-22-6,Yes,Mandatory,,,The value of a pointer to a FILE shall not be used after the associated stream has been closed,FIO46-C,IO1,Import, -c,MISRA-C-2012,RULE-22-7,Yes,Required,,,The macro EOF shall only be compared with the unmodified return value from any Standard Library function capable of returning EOF,,IO,Hard, +c,MISRA-C-2012,RULE-22-7,Yes,Required,,,The macro EOF shall only be compared with the unmodified return value from any Standard Library function capable of returning EOF,,IO3,Hard, c,MISRA-C-2012,RULE-22-8,Yes,Required,,,The value of errno shall be set to zero prior to a call to an errno- setting-function,ERR30-C,Contracts,Import, c,MISRA-C-2012,RULE-22-9,Yes,Required,,,The value of errno shall be tested against zero after calling an errno-setting-function,,Contracts,Medium, c,MISRA-C-2012,RULE-22-10,Yes,Required,,,The value of errno shall only be tested when the last function to be called was an errno-setting-function,,Contracts,Medium, diff --git a/scripts/generate_rules/generate_package_files.py b/scripts/generate_rules/generate_package_files.py index 3830f1fb3..aee6e145c 100644 --- a/scripts/generate_rules/generate_package_files.py +++ b/scripts/generate_rules/generate_package_files.py @@ -78,6 +78,16 @@ default=False, help="create anonymized versions of the queries, without identifying rule information", ) +# Skip the generation of tests. This is useful when creating releases +# wherein we should preserve the author's intention to not provide c-specific +# test cases. +parser.add_argument( + "--skip-shared-test-generation", + action="store_true", + dest="skip_shared_test_generation", + default=False, + help="Do not generate tests.", +) parser.add_argument("language_name", help="the language of the package") parser.add_argument( "package_name", help="the name of the package to generate query files for") @@ -105,7 +115,7 @@ -def write_shared_implementation(package_name, rule_id, query, language_name, ql_language_name, common_src_pack_dir, common_test_pack_dir): +def write_shared_implementation(package_name, rule_id, query, language_name, ql_language_name, common_src_pack_dir, common_test_pack_dir, skip_tests=False): shared_impl_dir_name = query["shared_implementation_short_name"].lower() @@ -146,50 +156,51 @@ def write_shared_implementation(package_name, rule_id, query, language_name, ql_ # Write out the test. Test are always stored under the `language_name` # directory. - shared_impl_test_dir = common_test_pack_dir.joinpath( - "rules", - shared_impl_dir_name - ) + if not skip_tests: + shared_impl_test_dir = common_test_pack_dir.joinpath( + "rules", + shared_impl_dir_name + ) - shared_impl_test_dir.mkdir(exist_ok=True, parents=True) + shared_impl_test_dir.mkdir(exist_ok=True, parents=True) - # Generate test query file - shared_impl_test_query_path = shared_impl_test_dir.joinpath( - f"{query['shared_implementation_short_name']}.ql" - ) - - with open(shared_impl_test_query_path, "w", newline="\n") as f: - f.write("// GENERATED FILE - DO NOT MODIFY\n") - f.write( - "import " - + str(shared_impl_query_library_path.relative_to(common_src_pack_dir).with_suffix('')) - .replace("\\", "/") - .replace("/", ".") - + "\n" + # Generate test query file + shared_impl_test_query_path = shared_impl_test_dir.joinpath( + f"{query['shared_implementation_short_name']}.ql" ) + + with open(shared_impl_test_query_path, "w", newline="\n") as f: + f.write("// GENERATED FILE - DO NOT MODIFY\n") + f.write( + "import " + + str(shared_impl_query_library_path.relative_to(common_src_pack_dir).with_suffix('')) + .replace("\\", "/") + .replace("/", ".") + + "\n" + ) - # Create an empty test file, if one doesn't already exist - shared_impl_test_dir.joinpath( - "test." + language_name).touch() + # Create an empty test file, if one doesn't already exist + shared_impl_test_dir.joinpath( + "test." + language_name).touch() - # Add an empty expected results file - this makes it possible to see the results the - # first time you run the test in VS Code - expected_results_file = shared_impl_test_dir.joinpath( - query["shared_implementation_short_name"] + ".expected") - if not expected_results_file.exists(): - with open(expected_results_file, "w", newline="\n") as f: - f.write( - "No expected results have yet been specified") + # Add an empty expected results file - this makes it possible to see the results the + # first time you run the test in VS Code + expected_results_file = shared_impl_test_dir.joinpath( + query["shared_implementation_short_name"] + ".expected") + if not expected_results_file.exists(): + with open(expected_results_file, "w", newline="\n") as f: + f.write( + "No expected results have yet been specified") - # Add a testref file for this query, that refers to the shared library - test_ref_file = test_src_dir.joinpath( - query["short_name"] + ".testref") + # Add a testref file for this query, that refers to the shared library + test_ref_file = test_src_dir.joinpath( + query["short_name"] + ".testref") - # don't write it if it already exists - if not test_ref_file.exists(): - with open(test_ref_file, "w", newline="\n") as f: - f.write(str(shared_impl_test_query_path.relative_to( - repo_root)).replace("\\", "/")) + # don't write it if it already exists + if not test_ref_file.exists(): + with open(test_ref_file, "w", newline="\n") as f: + f.write(str(shared_impl_test_query_path.relative_to( + repo_root)).replace("\\", "/")) def write_non_shared_testfiles(query, language_name, query_path, test_src_dir, src_pack_dir): @@ -360,7 +371,7 @@ def write_non_shared_testfiles(query, language_name, query_path, test_src_dir, s f"""standard-example.{query["sourcefile_ext"]}""").touch() if "shared_implementation_short_name" in query: - write_shared_implementation(package_name, rule_id, query, language_name, ql_language_name, common_src_pack_dir, common_test_pack_dir) + write_shared_implementation(package_name, rule_id, query, language_name, ql_language_name, common_src_pack_dir, common_test_pack_dir, args.skip_shared_test_generation) else: write_non_shared_testfiles(query, language_name, query_path, test_src_dir, src_pack_dir) # Exclusions diff --git a/scripts/generate_rules/templates/exclusions.qll.template b/scripts/generate_rules/templates/exclusions.qll.template index 2016871ea..25d192772 100644 --- a/scripts/generate_rules/templates/exclusions.qll.template +++ b/scripts/generate_rules/templates/exclusions.qll.template @@ -3,11 +3,15 @@ import cpp import RuleMetadata import codingstandards.cpp.exclusions.RuleMetadata +{% if data | length == 1 %} +newtype {{ package_name }}Query = T{{ data[0]['queryname']}}Query() +{% else %} newtype {{ package_name }}Query = {% for query in data%} T{{ query['queryname']}}Query(){% if not loop.last %} or {% endif %}{% endfor %} +{% endif %} predicate is{{package_name}}QueryMetadata(Query query, string queryId, string ruleId) { {% for item in data %}