InvalidMemory2: Implement EXP35-C query

Nikita Kraiouchkine · Nikita Kraiouchkine · commit 888b054c261a · 2023-03-28T01:26:41.000+02:00
diff --git a/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.md b/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.md
@@ -0,0 +1,225 @@
+# EXP35-C: Do not modify objects with temporary lifetime
+
+This query implements the CERT-C rule EXP35-C:
+
+> Do not modify objects with temporary lifetime
+
+
+## Description
+
+The C11 Standard \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\] introduced a new term: *temporary lifetime*. Modifying an object with temporary lifetime is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior). According to subclause 6.2.4, paragraph 8
+
+> A non-lvalue expression with structure or union type, where the structure or union contains a member with array type (including, recursively, members of all contained structures and unions) refers to an object with automatic storage duration and *temporary*lifetime. Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends when the evaluation of the containing full expression or full declarator ends. Any attempt to modify an object with temporary lifetime results in undefined behavior.
+
+
+This definition differs from the C99 Standard (which defines modifying the result of a function call or accessing it after the next sequence point as undefined behavior) because a temporary object's lifetime ends when the evaluation containing the full expression or full declarator ends, so the result of a function call can be accessed. This extension to the lifetime of a temporary also removes a quiet change to C90 and improves compatibility with C++.
+
+C functions may not return arrays; however, functions can return a pointer to an array or a `struct` or `union` that contains arrays. Consequently, in any version of C, if a function call returns by value a `struct` or `union` containing an array, do not modify those arrays within the expression containing the function call. In C99 and older, do not access an array returned by a function after the next sequence point or after the evaluation of the containing full expression or full declarator ends.
+
+## Noncompliant Code Example
+
+This noncompliant code example [conforms](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-conformingprogram) to the C11 Standard; however, it fails to conform to C99. If compiled with a C99-conforming implementation, this code has [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior) because the sequence point preceding the call to `printf()` comes between the call and the access by `printf()` of the string in the returned object.
+
+```cpp
+#include <stdio.h>
+
+struct X { char a[8]; };
+
+struct X salutation(void) {
+  struct X result = { "Hello" };
+  return result;
+}
+
+struct X addressee(void) {
+  struct X result = { "world" };
+  return result;
+}
+
+int main(void) {
+  printf("%s, %s!\n", salutation().a, addressee().a);
+  return 0;
+}
+
+```
+
+## Compliant Solution (C11 and newer)
+
+This compliant solution checks `__STDC_VERSION__` to ensure that a pre-C11 compiler will fail to compile the code, rather than invoking undefined behavior.
+
+```cpp
+#include <stdio.h>
+
+#if __STDC_VERSION__ < 201112L
+#error This code requires a compiler supporting the C11 standard or newer
+#endif
+
+struct X { char a[8]; };
+
+struct X salutation(void) {
+  struct X result = { "Hello" };
+  return result;
+}
+
+struct X addressee(void) {
+  struct X result = { "world" };
+  return result;
+}
+
+int main(void) {
+  printf("%s, %s!\n", salutation().a, addressee().a);
+  return 0;
+}
+```
+
+## Compliant Solution
+
+This compliant solution stores the structures returned by the call to `addressee()` before calling the `printf()` function. Consequently, this program conforms to both C99 and C11.
+
+```cpp
+#include <stdio.h>
+
+struct X { char a[8]; };
+ 
+struct X salutation(void) {
+  struct X result = { "Hello" };
+  return result;
+}
+
+struct X addressee(void) {
+  struct X result = { "world" };
+  return result;
+}
+
+int main(void) {
+  struct X my_salutation = salutation();
+  struct X my_addressee = addressee();
+ 
+  printf("%s, %s!\n", my_salutation.a, my_addressee.a);
+  return 0;
+}
+
+```
+
+## Noncompliant Code Example
+
+This noncompliant code example attempts to retrieve an array and increment the array's first value. The array is part of a `struct` that is returned by a function call. Consequently, the array has temporary lifetime, and modifying the array is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior) in both C99 and C11.
+
+```cpp
+#include <stdio.h>
+
+struct X { int a[6]; };
+
+struct X addressee(void) {
+  struct X result = { { 1, 2, 3, 4, 5, 6 } };
+  return result;
+}
+
+int main(void) {
+  printf("%x", ++(addressee().a[0]));
+  return 0;
+}
+
+```
+
+## Compliant Solution
+
+This compliant solution stores the structure returned by the call to `addressee()` as `my_x` before calling the `printf()` function. When the array is modified, its lifetime is no longer temporary but matches the lifetime of the block in `main()`.
+
+```cpp
+#include <stdio.h>
+
+struct X { int a[6]; };
+
+struct X addressee(void) {
+  struct X result = { { 1, 2, 3, 4, 5, 6 } };
+  return result;
+}
+
+int main(void) {
+  struct X my_x = addressee();
+  printf("%x", ++(my_x.a[0]));
+  return 0;
+}
+
+```
+
+## Noncompliant Code Example
+
+This noncompliant code example attempts to save a pointer to an array that is part of a `struct` that is returned by a function call. Consequently, the array has temporary lifetime, and using the pointer to it outside of the full expression is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior) in both C99 and C11.
+
+```cpp
+#include <stdio.h>
+
+struct X { int a[6]; };
+
+struct X addressee(void) {
+  struct X result = { { 1, 2, 3, 4, 5, 6 } };
+  return result;
+}
+
+int main(void) {
+  int *my_a = addressee().a;
+  printf("%x", my_a[0]);
+  return 0;
+}
+
+```
+
+## Compliant Solution
+
+This compliant solution stores the structure returned by the call to `addressee()` as `my_x` before saving a pointer to its array member. When the pointer is used, its lifetime is no longer temporary but matches the lifetime of the block in `main()`.
+
+```cpp
+#include <stdio.h>
+
+struct X { int a[6]; };
+
+struct X addressee(void) {
+  struct X result = { { 1, 2, 3, 4, 5, 6 } };
+  return result;
+}
+
+int main(void) {
+  struct X my_x = addressee();
+  int *my_a = my_x.a;
+  printf("%x", my_a[0]);
+  return 0;
+}
+
+```
+
+## Risk Assessment
+
+Attempting to modify an array or access it after its lifetime expires may result in erroneous program behavior.
+
+<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> EXP35-C </td> <td> Low </td> <td> Probable </td> <td> Medium </td> <td> <strong>P4</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>
+
+
+## Automated Detection
+
+<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> <strong>temporary-object-modification</strong> </td> <td> Partially checked </td> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-EXP35</strong> </td> <td> </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.4 </td> <td> <strong>C0450, C0455, C0459, C0464, C0465</strong> <strong>C++3807, C++3808</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>642 S, 42 D, 77 D</strong> </td> <td> Enhanced Enforcement </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.2 </td> <td> <strong>CERT_C-EXP35-a</strong> </td> <td> Do not modify objects with temporary lifetime </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2023a </td> <td> <a> CERT-C: Rule EXP35-C </a> </td> <td> Checks for accesses on objects with temporary lifetime (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>0450 \[U\], 0455 \[U\], 0459 \[U\],</strong> <strong>0464 \[U\], 0465 \[U\]</strong> </td> <td> </td> </tr> <tr> <td> <a> Splint </a> </td> <td> 3.1.1 </td> <td> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> <strong>temporary-object-modification</strong> </td> <td> Partially checked </td> </tr> </tbody> </table>
+
+
+## Related Vulnerabilities
+
+Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+EXP35-C).
+
+## Related Guidelines
+
+[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
+
+<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> ISO/IEC TR 24772:2013 </a> </td> <td> Dangling References to Stack Frames \[DCM\] </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> ISO/IEC TR 24772:2013 </a> </td> <td> Side-effects and Order of Evaluation \[SAM\] </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>
+
+
+## Bibliography
+
+<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 6.2.4, "Storage Durations of Objects" </td> </tr> </tbody> </table>
+
+
+## Implementation notes
+
+This implementation does not cover modification or access of the result of a function call after the next sequence point, which is undefined behavior only pre-C11.
+
+## References
+
+* CERT-C: [EXP35-C: Do not modify objects with temporary lifetime](https://wiki.sei.cmu.edu/confluence/display/c)
diff --git a/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql b/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql
@@ -0,0 +1,38 @@
+/**
+ * @id c/cert/do-not-modify-objects-with-temporary-lifetime
+ * @name EXP35-C: Do not modify objects with temporary lifetime
+ * @description Attempting to modify an object with temporary lifetime results in undefined
+ *              behavior.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/exp35-c
+ *       correctness
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+
+/**
+ * A struct or union type that contains an array type
+ */
+class StructOrUnionTypeWithArrayField extends Struct {
+  StructOrUnionTypeWithArrayField() {
+    this.getAField().getUnspecifiedType() instanceof ArrayType
+    or
+    // nested struct or union containing an array type
+    this.getAField().getUnspecifiedType().(Struct) instanceof StructOrUnionTypeWithArrayField
+  }
+}
+
+// Note: Undefined behavior is possible regardless of whether the accessed field from the returned
+// struct is an array or a scalar (i.e. arithmetic and pointer types) member, according to the standard.
+from FieldAccess fa, FunctionCall fc
+where
+  not isExcluded(fa, InvalidMemory2Package::doNotModifyObjectsWithTemporaryLifetimeQuery()) and
+  not fa.getQualifier().isLValue() and
+  fa.getQualifier().getUnconverted() = fc and
+  fa.getQualifier().getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField
+select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", fc,
+  "function call"
diff --git a/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected b/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected
@@ -0,0 +1,4 @@
+| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | function call |
+| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | function call |
+| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | function call |
+| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | function call |
diff --git a/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.qlref b/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.qlref
@@ -0,0 +1 @@
+rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql
diff --git a/c/cert/test/rules/EXP35-C/test.c b/c/cert/test/rules/EXP35-C/test.c
@@ -0,0 +1,71 @@
+#include <stdlib.h>
+
+typedef float AF12[12];
+
+struct S1 {
+  char a[8];
+};
+struct S2 {
+  struct S1 *s1;
+};
+struct S3 {
+  struct S1 s1;
+  int i1;
+};
+struct S4 {
+  AF12 af12;
+};
+
+struct S5 {
+  int i1;
+  struct S1 *s1;
+};
+
+struct S1 get_s1(void) {
+  struct S1 s1;
+  return s1;
+}
+
+struct S1 *get_s1_ptr(void) {
+  struct S1 *s1 = malloc(sizeof(struct S1));
+  return s1;
+}
+
+struct S2 get_s2(void) {
+  struct S2 s2;
+  return s2;
+}
+
+struct S3 get_s3(void) {
+  struct S3 s3;
+  return s3;
+}
+
+struct S4 get_s4(void) {
+  struct S4 s4;
+  return s4;
+}
+
+struct S5 get_s5(void) {
+  struct S5 s5;
+  return s5;
+}
+
+void test_field_access(void) {
+  struct S1 s1 = get_s1();
+  struct S2 s2 = get_s2();
+  struct S3 s3 = get_s3();
+  struct S4 s4 = get_s4();
+
+  s1.a[0] = 'a';     // COMPLIANT
+  s2.s1->a[0] = 'a'; // COMPLIANT
+  s3.s1.a[0] = 'a';  // COMPLIANT
+  s4.af12[0] = 0.0f; // COMPLIANT
+
+  (void)get_s1().a;     // NON_COMPLIANT
+  (void)get_s2().s1->a; // COMPLIANT
+  (void)get_s3().s1.a;  // NON_COMPLIANT
+  (void)get_s3().i1;    // NON_COMPLIANT - even if scalar type accessed
+  (void)get_s4().af12;  // NON_COMPLIANT
+  (void)get_s5().s1->a; // COMPLIANT
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql`