Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

STR34-C: Do not consider integer type aliases in templates #635

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

STR34-C: Do not consider integer type aliases in templates #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87fa7c1
STR34-C: Do not consider integer type aliases in templates
mbaluda Jul 5, 2024
dc6bed1
Add change notes
mbaluda Jul 5, 2024
e3a5b8c
renamed the shared query
mbaluda Jul 25, 2024
946d5df
Fix formatting
mbaluda Jul 25, 2024
f154426
Merge branch 'main' into mabluda/fix576
mbaluda Jul 25, 2024
ed381ef
Merge branch 'main' into mabluda/fix576
mbaluda Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions c/cert/src/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,11 @@

import cpp
import codingstandards.c.cert
import semmle.code.cpp.commons.CommonType
import codingstandards.cpp.rules.castcharbeforeconvertingtolargersizes.CastCharBeforeConvertingToLargerSizes

from Cast c
where
not isExcluded(c, Strings3Package::castCharBeforeConvertingToLargerSizesQuery()) and
// find cases where there is a conversion happening wherein the
// base type is a char
c.getExpr().getType() instanceof CharType and
not c.getExpr().getType() instanceof UnsignedCharType and
// it's a bigger type
c.getType().getSize() > c.getExpr().getType().getSize() and
// and it's some kind of integer type
c.getType() instanceof IntegralType
select c.getExpr(),
"Expression not converted to `unsigned char` before converting to a larger integer type."
class CastCharBeforeConvertingToLargerSizesQuery extends CastCharBeforeConvertingToLargerSizesSharedQuery
{
CastCharBeforeConvertingToLargerSizesQuery() {
this = Strings3Package::castCharBeforeConvertingToLargerSizesQuery()
}
}
21 changes: 0 additions & 21 deletions c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.qlref
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.testref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c/common/test/rules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.ql
21 changes: 21 additions & 0 deletions ...ules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
| test.c:9:7:9:14 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:30:11:30:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:31:3:31:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:31:11:31:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:33:11:33:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:34:11:34:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:35:3:35:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:35:11:35:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:36:3:36:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:36:11:36:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:37:3:37:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:37:11:37:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:38:3:38:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:38:11:38:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:39:11:39:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:11:40:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:41:3:41:13 | (unsigned int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:41:11:41:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:12:42:13 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:44:11:44:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:45:11:45:12 | (int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
14 changes: 7 additions & 7 deletions ...oreConvertingToLargerSizes.expected.clang → ...oreConvertingToLargerSizes.expected.clang
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
| test.c:7:7:7:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:28:3:28:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:29:3:29:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:9:7:9:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:30:3:30:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:31:3:31:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:32:3:32:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:33:3:33:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:34:3:34:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:35:3:35:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:36:3:36:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:37:3:37:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:38:3:38:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:39:3:39:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:11:42:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:43:11:43:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:41:3:41:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:3:42:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:44:11:44:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:45:11:45:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
12 changes: 6 additions & 6 deletions ...eforeConvertingToLargerSizes.expected.gcc → ...eforeConvertingToLargerSizes.expected.gcc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
| test.c:7:7:7:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:28:3:28:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:29:3:29:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:9:7:9:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:30:3:30:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:31:3:31:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:32:3:32:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:33:3:33:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:34:3:34:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:35:3:35:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:36:3:36:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:37:3:37:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:38:3:38:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:39:3:39:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:11:42:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:41:3:41:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:3:42:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:43:11:43:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:44:11:44:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
14 changes: 7 additions & 7 deletions ...eforeConvertingToLargerSizes.expected.qcc → ...eforeConvertingToLargerSizes.expected.qcc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
| test.c:7:7:7:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:28:3:28:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:29:3:29:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:9:7:9:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:30:3:30:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:31:3:31:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:32:3:32:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:33:3:33:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:34:3:34:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:35:3:35:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:36:3:36:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:37:3:37:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:38:3:38:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:39:3:39:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:3:42:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:43:3:43:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:40:3:40:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:41:3:41:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:42:3:42:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:44:3:44:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.c:45:3:45:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. |
4 changes: 4 additions & 0 deletions ...test/rules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// GENERATED FILE - DO NOT MODIFY
import codingstandards.cpp.rules.castcharbeforeconvertingtolargersizes.CastCharBeforeConvertingToLargerSizes

class TestFileQuery extends CastCharBeforeConvertingToLargerSizesSharedQuery, TestQuery { }
2 changes: 2 additions & 0 deletions c/cert/test/rules/STR34-C/test.c → ...tcharbeforeconvertingtolargersizes/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// NOTICE: THE TEST CASES BELOW ARE ALSO INCLUDED IN THE C++ TEST CASE AND
// CHANGES SHOULD BE REFLECTED THERE AS WELL.
#include <ctype.h>
#include <stdio.h>

Expand Down
2 changes: 2 additions & 0 deletions change_notes/2024-07-05-fix-fp-576-STR34-C.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `STR34-C` - `CastCharBeforeConvertingToLargerSizes.ql`:
- Fixes #576. Do not consider integer type aliases in templates.
26 changes: 26 additions & 0 deletions ...cpp/rules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Provides a library which includes a `problems` predicate for reporting....
*/

import cpp
import codingstandards.cpp.Customizations
import codingstandards.cpp.Exclusions

abstract class CastCharBeforeConvertingToLargerSizesSharedQuery extends Query { }

Query getQuery() { result instanceof CastCharBeforeConvertingToLargerSizesSharedQuery }

query predicate problems(Cast c, string message) {
not isExcluded(c, getQuery()) and
// find cases where there is a conversion happening wherein the
// base type is a char
c.getExpr().getType() instanceof CharType and
not c.getExpr().getType() instanceof UnsignedCharType and
// it's a bigger type
c.getType().getSize() > c.getExpr().getType().getSize() and
// and it's some kind of integer type
c.getType().getUnderlyingType() instanceof IntegralType and
not c.isFromTemplateInstantiation(_) and
message =
"Expression not converted to `unsigned char` before converting to a larger integer type."
}
2 changes: 2 additions & 0 deletions ...ules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
| test.cpp:11:9:11:9 | (int32_t)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
| test.cpp:12:41:12:41 | (signed int)... | Expression not converted to `unsigned char` before converting to a larger integer type. |
4 changes: 4 additions & 0 deletions ...test/rules/castcharbeforeconvertingtolargersizes/CastCharBeforeConvertingToLargerSizes.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// GENERATED FILE - DO NOT MODIFY
import codingstandards.cpp.rules.castcharbeforeconvertingtolargersizes.CastCharBeforeConvertingToLargerSizes

class TestFileQuery extends CastCharBeforeConvertingToLargerSizesSharedQuery, TestQuery { }
17 changes: 17 additions & 0 deletions cpp/common/test/rules/castcharbeforeconvertingtolargersizes/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// NOTICE: THE TEST CASES BELOW ARE ALSO INCLUDED IN THE C TEST CASE AND
// CHANGES SHOULD BE REFLECTED THERE AS WELL.
#include <cstdint>

template <typename S, typename T> S get(T t) {
S s = t; // COMPLIANT
return s;
}

void test(std::int32_t i32, std::int8_t i8, char c) {
i32 = c; // NON_COMPLIANT
i32 = get<std::int32_t, std::int32_t>(c); // NON_COMPLIANT
i32 = get<std::int32_t, std::int8_t>(c); // COMPLIANT
i32 = i8; // COMPLIANT
i32 = get<std::int32_t, std::int32_t>(i8); // COMPLIANT
i32 = get<std::int32_t, std::int8_t>(i8); // COMPLIANT
}
1 change: 1 addition & 0 deletions rule_packages/c/Strings3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"precision": "very-high",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the rule is shared between c and c++, doesnt it need to have the shared implementation name portion in the package description file for both c/c++?

for example like this?

Copy link
Contributor Author

@mbaluda mbaluda Jul 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no c++ rule using this query.
It's just that the issue is related to using the query on c++ code, so I needed a c++ test.
That's why I had to move things around

"severity": "error",
"short_name": "CastCharBeforeConvertingToLargerSizes",
"shared_implementation_short_name": "CastCharBeforeConvertingToLargerSizes",
"tags": [
"correctness",
"security"
Expand Down
Loading