-
Notifications
You must be signed in to change notification settings - Fork 265
Description
Suggestion
Cppfront could report errors when detecting implicit numeric truncation and/or conversion in function return statements.
These would be compile-time only checks for:
- numeric literals having invalid values for the function's return type
- expressions having a type incompatible with the function's return type
Examples:
t1: () -> uchar = {
return -1; // Invalid literal (signed/unsigned)
}
t2: () -> char = {
return 300; // Invalid literal (out-of-range for type)
}
t3: () -> i32 = {
return 4'000'000'000; // Invalid literal (out-of-range for type)
}
t4: () -> i32 = {
return 1.0; // Invalid literal (not an integer literal)
}
t5_helper: () -> i32 = {
return std::numeric_limits<int>::min();
}
t5: () -> ushort = {
return t5_helper(); // t5_helper returns i32, incompatible with ushort
}
Will your feature suggestion eliminate X% of security vulnerabilities of a given kind in current C++ code?
Yes, see:
- CWE-197: Numeric Truncation Error
- CWE-681: Incorrect Conversion between Numeric Types
- CWE-195: Signed to Unsigned Conversion Error.
(There are various CVEs listed for these weaknesses but I couldn't find any with source code.)
Here's a Cpp2 snippet based on examples from the above CWE links:
enum {
kError_InvalidDimensions = -1
};
get_image_data_size: (width: u32, height: u32) -> u32 = {
if (width > 1024 && height > 1024)
{
return width * height;
}
else if (width > 512)
{
return width;
}
else if (width > 256)
{
return width / 2;
}
else
{
return kError_InvalidDimensions; // Oops, programmer bug
}
}
copy_image_data: (dst: *char, src: *const char, width: u32, height: u32) = {
std::memcpy(dst, src, get_image_data_size(width, height));
}
Will your feature suggestion automate or eliminate X% of current C++ guidance literature?
Yes, it automates the guidance to compile at high warning levels. See:
- Sutter & Alexandrescu, C++ Coding Standards: 101 Rules, Guidelines, and Best Practices (Item 1. Compile cleanly at high warning levels)
- Jason Turner, C++ Weekly - Ep 353 - Implicit Conversions Are Evil
Using the above Cpp2 example, there are no warnings from the 3 major compilers about return kError_InvalidDimensions with default options. To produce a warning:
- MSVC 19: requires
/W4 - Clang 15: requires
-Wconversion
I couldn't get a warning in GCC 12.2 even with -Wall -Wextra -Wpedantic -Wconversion -Wsign-conversion
https://godbolt.org/z/M9bb46j4T
Describe alternatives you've considered.
One alternative is to provide runtime checks which would validate the actual value rather than just the type itself.
get_internal_count: () -> i32 = {
return 5;
}
// This is fine; the value 5 returned as an i32 can fit inside uchar without any truncation or signed conversion.
get_usage_count: () -> uchar = {
return cpp2_runtime_check<uchar>(get_internal_count());
}
However, this could possibly introduce a significant runtime overhead for functions called in hot loops, and would probably require an opt-in/out preference, whereas the suggestion above is for compile-time only checks.