From f01737a4c01412d14c793c46a10538ff601fb8e6 Mon Sep 17 00:00:00 2001 From: M Starch Date: Tue, 11 Mar 2025 14:56:57 -0700 Subject: [PATCH 1/3] Fixing BasicIntTypes to allow C Standard Integers and 'bool' The purpose of this check is to ensure that all integral types used by the code point to some fixed size type (e.g. an unsigned 8-bit integer). However; the previous implementation only allowed JPL style typedefs (i.e. U8) and ignored C standard integer types (i.e. uint8_t). This causes the query to false-positive when a typedef resolves to a C standard int type. 'bool' has also be allowed as part of the exclusions list as it represents distinct values 'true' and 'false' in C++ code. --- cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql | 7 ++++--- cpp/ql/src/change-notes/2025-03-11-basic-int-types.md | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 cpp/ql/src/change-notes/2025-03-11-basic-int-types.md diff --git a/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql b/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql index 82b7f146554..614980fa9a1 100644 --- a/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql +++ b/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql @@ -12,7 +12,8 @@ import cpp predicate allowedTypedefs(TypedefType t) { - t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32"] + t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32", + "int64_t", "uint64_t", "int32_t", "uint32_t", "int16_t", "uint16_t", "int8_t", "uint8_t"] } /** @@ -38,8 +39,8 @@ Type getAUsedType(Type t) { } predicate problematic(IntegralType t) { - // List any exceptions that should be allowed. - any() + // 'bool' is allowed as it represents a 'true' or 'false' value + t.getName() != ["bool"] } from Declaration d, Type usedType diff --git a/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md b/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md new file mode 100644 index 00000000000..6c6119f06b3 --- /dev/null +++ b/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* The query "Basic Integral Types" in JPL_C has been updated to allow C standard integer types (uint8_t etc.) and 'bool'. \ No newline at end of file From 7f4905987efc00cdb4dbab6fad291e2789c2b654 Mon Sep 17 00:00:00 2001 From: M Starch Date: Thu, 13 Mar 2025 11:12:35 -0700 Subject: [PATCH 2/3] Addressing review comments Reduced the category to minorAnalysis. Handled bools via a instanceof with BoolType. Formatted the query correctly. --- cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql | 13 +++++++++---- .../src/change-notes/2025-03-11-basic-int-types.md | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql b/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql index 614980fa9a1..16ac18b87cc 100644 --- a/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql +++ b/cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql @@ -12,8 +12,11 @@ import cpp predicate allowedTypedefs(TypedefType t) { - t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32", - "int64_t", "uint64_t", "int32_t", "uint32_t", "int16_t", "uint16_t", "int8_t", "uint8_t"] + t.getName() = + [ + "I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32", "int64_t", "uint64_t", + "int32_t", "uint32_t", "int16_t", "uint16_t", "int8_t", "uint8_t" + ] } /** @@ -39,14 +42,16 @@ Type getAUsedType(Type t) { } predicate problematic(IntegralType t) { - // 'bool' is allowed as it represents a 'true' or 'false' value - t.getName() != ["bool"] + // List any exceptions that should be allowed. + any() } from Declaration d, Type usedType where usedType = getAUsedType*(getAnImmediateUsedType(d)) and problematic(usedType) and + // Allow uses of boolean types where defined by the language. + not usedType instanceof BoolType and // Ignore violations for which we do not have a valid location. not d.getLocation() instanceof UnknownLocation select d, diff --git a/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md b/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md index 6c6119f06b3..7d3bd4b8233 100644 --- a/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md +++ b/cpp/ql/src/change-notes/2025-03-11-basic-int-types.md @@ -1,4 +1,4 @@ --- -category: majorAnalysis +category: minorAnalysis --- -* The query "Basic Integral Types" in JPL_C has been updated to allow C standard integer types (uint8_t etc.) and 'bool'. \ No newline at end of file +* The query "Use of basic integral type" (`cpp/jpl-c/basic-int-types`) no longer produces alerts for the standard fixed width integer types (`int8_t`, `uint8_t`, etc.), and the `_Bool` and `bool` types. \ No newline at end of file From 7b5d604607646a2e97e79a1f5cb4cf8c066a75c6 Mon Sep 17 00:00:00 2001 From: M Starch Date: Thu, 13 Mar 2025 15:04:37 -0700 Subject: [PATCH 3/3] Updating tests to allow new typedefs --- .../test/query-tests/JPL_C/LOC-3/Rule 17/BasicIntTypes.expected | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 17/BasicIntTypes.expected b/cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 17/BasicIntTypes.expected index 0664ca9e369..58facbeac8e 100644 --- a/cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 17/BasicIntTypes.expected +++ b/cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 17/BasicIntTypes.expected @@ -1,3 +1 @@ | test.c:6:26:6:26 | x | x uses the basic integral type unsigned char rather than a typedef with size and signedness. | -| test.c:7:20:7:20 | x | x uses the basic integral type unsigned char rather than a typedef with size and signedness. | -| test.c:10:16:10:20 | test7 | test7 uses the basic integral type unsigned char rather than a typedef with size and signedness. |