Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

Many functions require the allocation of multiple objectsresources. Failing and returning somewhere in the middle of this function without freeing all of the allocated memory resources could produce a memory leak. It is a common error to forget to free one (or all) of the objects resources in this manner, so a goto-chain is the simplest and cleanest way to organize exits when order is preserved.

Noncompliant Code Example

In this noncompliant example, exit code is written for every instance in which the function can terminate prematurely. Notice how failing to allocate obj3 produces a memory leak and fails to close the opened file.

Code Block
bgColor#FFCCCC

typedef struct object {   // A generic struct -- The contents don't matter
  int propertyA, propertyB, propertyC;
} object_t;


int do_something(void){
  // ... definitions ...
  FILE *fin;
  object_t *obj1, *obj2, *obj3;
  
  fin = fopen("some_file", "r");
  if (fin == NULL){
    return -1;
  }

  obj1 = allocate_object(malloc(sizeof(object_t));
  if (obj1 == NULL){
    fclose(fin);
    return -1;
  }

  obj2 = allocate_object(malloc(sizeof(object_t));
  if (obj2 == NULL){
    fclose(fin);
    free(obj1);
    return -1;
  }

  obj3 = allocate_object(malloc(sizeof(object_t));
  if (obj3 == NULL){
    fclose(fin);
    free(obj2);
    return -1; // Forgot to free obj1 -- Memory leak
  }

  // ... more code ...
}

This is also just a small example; in much larger examples, errors like this would be even harder to detect.

Compliant Solution

In this revised version, we have used a goto-chain in replacement of each individual return segment. If there is no error, control flow will fall through to the SUCCESS label and return 0. In the case of an error, control flow will jump to the proper failure label and the appropriate memory will be freed before returning an error.

Code Block
bgColor#CCCCFF
// ... assume the same struct as above ...

int do_something(void){
  FILE *fin;
  object_t *obj1, *obj2, *obj3;  

  fin = fopen("some_file", "r");
  if (fin == NULL){
    // ... definitions ...goto FAIL_FIN;
  }

  obj1 = allocate_object(malloc(sizeof(object_t));
  if (obj1 == NULL){
    goto FAIL_OBJ1;
  }

  obj2 = allocate_object(malloc(sizeof(object_t));
  if (obj2 == NULL){
    goto FAIL_OBJ2;
  }

  obj3 = allocate_object(malloc(sizeof(object_t));
  if (obj3 == NULL){
    goto FAIL_OBJ3;
  }


  // ... more code ...


SUCCESS:     // Return normally
  return 0;

FAIL_OBJ3:   // Otherwise, free objects in order
  free(obj2);

FAIL_OBJ2:
  free(obj1);

FAIL_OBJ1:
  fclose(fin);

FAIL_FIN:
  return -1;
}

The benefits of this method are that the code is cleaner and we prevent the rewriting of similar code upon every function error.

Risk Assessment

Failure to free allocated memory or close opened files results in a memory leak and possibly unexpected results.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

MEM12-C

low

probable

medium

P3

L3

References

Wiki Markup
\[[ISO/IEC 9899:1999|AA. C References#ISO/IEC 9899-1999]\] Section 7.20.3, "Memory management functions"
\[[ISO/IEC 9899:1999|AA. C References#ISO/IEC 9899-1999]\] Section 7.19.5, "File access functions"
\[[Seacord 05|AA. C References#Seacord 05]\] Chapter 4, "Dynamic Memory Management"

Related Vulnerabilities

Note that this method does not only prevent leakage from memory allocation; the same logic could be used to ensure the closure of files and pipes, for example. Similarly, in In C++, this would also apply more generally to constructors and destructorsone could use this same strategy when constructing and destructing objects.