diff --git a/rules/S2107/cfamily/rule.adoc b/rules/S2107/cfamily/rule.adoc index c8192e4454..22b1df0597 100644 --- a/rules/S2107/cfamily/rule.adoc +++ b/rules/S2107/cfamily/rule.adoc @@ -1,4 +1,4 @@ -Partially-initialized objects are surprising to the `class` users +Partially initialized objects are surprising to the `class` users and might lead to hard-to-catch bugs. ``class``es with constructors are expected to have all members initialized after their constructor finishes. @@ -27,23 +27,23 @@ struct PartInit { }; ---- -This leads to undefined behavior in benign-looking code like on the example below. +This leads to undefined behavior in benign-looking code, like in the example below. In this particular case, garbage value may be printed, or a compiler may optimize away the print statement completely. [source,cpp] ---- PartInit pi(1); -std::cout <++`` header: +{cpp}20 introduced a remedy to this common pitfall: a family of ``++std::cmp_*++`` functions defined in the ``++++`` header: * ``++std::cmp_equal++`` * ``++std::cmp_not_equal++`` @@ -56,7 +56,7 @@ int main(int argc, char **argv) { if (user_input == 0xBEEF) { printf("Whoopsie daisy, ...\n"); // A malicious user can craft input arguments such that the flow of control - // passes through this call to `execl` which opens a new shell with this + // passes through this call to `execl`, which opens a new shell with this // program's (possibly elevated) permissions. execl("/bin/bash", "bash", (char *)NULL); } else { @@ -66,26 +66,26 @@ int main(int argc, char **argv) { } ---- -The program takes as arguments a string and its size, and uses these arguments to copy the string argument into an internal buffer. -Before copying the string into its internal buffer it checks whether the user-provided string fits into the buffer. -The program also comprises a call to `execl` that opens a shell with the program's possibly elevated permissions -- a potentially dangerous endeavour. -Even though the call to `execl` seems unreachable at a first glance, it can actually be reached due to signed/unsigned integer conversion. +The program takes a string and its size as arguments and uses these arguments to copy the string argument into an internal buffer. +Before copying the string into its internal buffer, it checks whether the user-provided string fits into the buffer. +The program also comprises a call to `execl` that opens a shell with the program's possibly elevated permissions -- a potentially dangerous endeavor. +Even though the call to `execl` seems unreachable at first glance, it can actually be reached due to signed/unsigned integer conversion. The check for the buffer size only validates that the provided string length (`user_input`) is smaller or equal to the buffer's size. Since the `atoi` function returns a signed integer, a user may provide a negative number to withstand that check. -The result of `sizeof(*)` on the other hand returns an unsigned integer which causes the expression `user_input * sizeof(char)` to be evaluated by +On the other hand, the result of `sizeof(*)` returns an unsigned integer which causes the expression `user_input * sizeof(char)` to be evaluated by . converting both operands to unsigned integers, . performing the multiplication, and . returning the result as an unsigned integer type. -A malicious user is hence able to provide carefully crafted negative integer and string to bypass the size check while still arriving at the appropriate size argument to not crash `memcpy`. -This, in turn, enables the malicious user to overflow the buffer variable `buf` to override the `user_input` variable which allows the second `if` statement to be evaluated to true, eventually opening a new shell with the target program's possibly elevated permissions. +Hence, a malicious user can provide carefully crafted negative integer and string to bypass the size check while still arriving at the appropriate size argument to not crash `memcpy`. +In turn, this enables the malicious user to overflow the buffer variable `buf` to override the `user_input` variable, which allows the second `if` statement to be evaluated to true, eventually opening a new shell with the target program's possibly elevated permissions. == How to fix it -Use the appropriate function from the ``++std::cmp_*++`` family to conduct comparisons between signed and unsigned integer types. +Use the appropriate function from the ``++std::cmp_*++`` family to compare signed and unsigned integer types. === Code examples @@ -131,10 +131,10 @@ bool fun(int x, std::vector const& v) { Note that this rule (S6183) deliberately avoids intersection with S6214. -While S6214 raises an issue if the signed value can be proven to be negative (in which case it is definitely a bug), S6281 will flag all *other* comparisons between signed and unsigned integers. +While S6214 raises an issue if the signed value can be proven to be negative (in which case it is definitely a bug), S6183 will flag all *other* comparisons between signed and unsigned integers. Therefore, if this rule is enabled, S6214 should be enabled too. -The following code snippet is hence compliant with S6183, but noncompliant with S6214 which will raise an issue on this definite bug. +The following code snippet is compliant with S6183 but noncompliant with S6214, which will raise an issue on this definite bug. [source,cpp,diff-id=3,diff-type=noncompliant] ---- diff --git a/rules/S946/cfamily/rule.adoc b/rules/S946/cfamily/rule.adoc index 5cff43b4e0..8d3da748a4 100644 --- a/rules/S946/cfamily/rule.adoc +++ b/rules/S946/cfamily/rule.adoc @@ -233,14 +233,14 @@ void caller() { * MISRA {cpp}:2008, 7-5-2 - The address of an object with automatic storage shall not be assigned to another object that may persist after the first object has ceased to exist * MISRA C:2012, 18.6 - The address of an object with automatic storage shall not be copied to another object that persists after the first object has ceased to exist -=== Related rules + +ifdef::env-github,rspecator-view[] + +=== Related rules but not implemented * S837 detects attempts to return addresses of automatic variables * S839 ensures that functions do not return references or pointers to parameters that are passed by reference - -ifdef::env-github,rspecator-view[] - ''' == Implementation Specification (visible only on this page) diff --git a/shared_content/cfamily/garbage_value_impact.adoc b/shared_content/cfamily/garbage_value_impact.adoc index f8a2094116..af9df3da57 100644 --- a/shared_content/cfamily/garbage_value_impact.adoc +++ b/shared_content/cfamily/garbage_value_impact.adoc @@ -3,7 +3,7 @@ Using garbage values will cause the program to behave nondeterministically at runtime. The program may produce a different output or crash depending on the run. -In some situations, loading a variable may expose a sensitive data, +In some situations, loading a variable may expose sensitive data, such as a password that was previously stored in the same location, leading to a vulnerability that uses such a defect as a gadget for extracting information from the instance