SEI CERT C Coding Standard
Space shortcuts
Page tree
Skip to end of metadata
Go to start of metadata

Before the lifetime of the last pointer that stores the return value of a call to a standard memory allocation function has ended, it must be matched by a call to free() with that pointer value.

Noncompliant Code Example

In this noncompliant example, the object allocated by the call to malloc() is not freed before the end of the lifetime of the last pointer text_buffer referring to the object:

#include <stdlib.h>
 
enum { BUFFER_SIZE = 32 };

int f(void) {
  char *text_buffer = (char *)malloc(BUFFER_SIZE); 
  if (text_buffer == NULL) {
    return -1;
  }
  return 0;
}

Compliant Solution

In this compliant solution, the pointer is deallocated with a call to free():

#include <stdlib.h>

enum { BUFFER_SIZE = 32 };

int f(void) {
  char *text_buffer = (char *)malloc(BUFFER_SIZE); 
  if (text_buffer == NULL) {
    return -1;
  }
 
  free(text_buffer);
  return 0;
}

Exceptions

MEM31-C-EX1: Allocated memory does not need to be freed if it is assigned to a pointer with static storage duration whose lifetime is the entire execution of a program. The following code example illustrates a pointer that stores the return value from malloc() in a static variable:

#include <stdlib.h>
 
enum { BUFFER_SIZE = 32 };

int f(void) {
  static char *text_buffer = NULL;
  if (text_buffer == NULL) {
    text_buffer = (char *)malloc(BUFFER_SIZE); 
    if (text_buffer == NULL) {
      return -1;
    }
  }
  return 0;
}

Risk Assessment

Failing to free memory can result in the exhaustion of system memory resources, which can lead to a denial-of-service attack.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

MEM31-C

Medium

Probable

Medium

P8

L2

Automated Detection

Tool

Version

Checker

Description

Astrée
19.04

Supported, but no explicit checker
Axivion Bauhaus Suite

6.9.0

CertC-MEM31Can detect dynamically allocated resources that are not freed
CodeSonar
5.2p0

ALLOC.LEAK

Leak

Compass/ROSE


Coverity

2017.07

RESOURCE_LEAK

ALLOC_FREE_MISMATCH

Finds resource leaks from variables that go out of scope while owning a resource

Cppcheck
 1.66
 leakReturnValNotUsedDoesn't use return value of memory allocation function
Klocwork
2018

MLK.MIGHT
MLK.MUST
MLK.RET.MUST
MLK.RET.MIGHT


LDRA tool suite
9.7.1

50 D

Partially implemented
Parasoft C/C++test
10.4.2

CERT_C-MEM31-a

Ensure resources are freed
Parasoft Insure++

Runtime analysis
Polyspace Bug Finder

R2019b

CERT C: Rule MEM31-CChecks for memory leak (rule fully covered)


PRQA QA-C

9.7

2706, 2707, 2708
PRQA QA-C++

4.4

2706, 2707, 2708, 3337, 3338
SonarQube C/C++ Plugin
3.11
S3584
Splint

3.1.1



TrustInSoft Analyzer

1.38

mallocExhaustively verified.

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

Key here (explains table format and definitions)

Taxonomy

Taxonomy item

Relationship

ISO/IEC TR 24772:2013Memory Leak [XYL]Prior to 2018-01-12: CERT: Unspecified Relationship
ISO/IEC TS 17961Failing to close files or free dynamic memory when they are no longer needed [fileclose]Prior to 2018-01-12: CERT: Unspecified Relationship
CWE 2.11CWE-401, Improper Release of Memory Before Removing Last Reference ("Memory Leak")2017-07-05: CERT: Exact
CWE 2.11CWE-4042017-07-06: CERT: Rule subset of CWE
CWE 2.11CWE-4592017-07-06: CERT: Rule subset of CWE
CWE 2.11CWE-7712017-07-06: CERT: Rule subset of CWE
CWE 2.11CWE-7722017-07-06: CERT: Rule subset of CWE

CERT-CWE Mapping Notes

Key here for mapping notes

CWE-404/CWE-459/CWE-771/CWE-772 and FIO42-C/MEM31-C

Intersection( FIO42-C, MEM31-C) = Ø

CWE-404 = CWE-459 = CWE-771 = CWE-772

CWE-404 = Union( FIO42-C, MEM31-C list) where list =

  • Failure to free resources besides files or memory chunks, such as mutexes)

Bibliography

[ISO/IEC 9899:2011]Subclause 7.22.3, "Memory Management Functions"



17 Comments

  1. Mark Dowd

    Maybe there needs to special mention of the UNIX realloc() gotcha here - if the size passed to realloc() is 0, realloc() frees the memory you pass, rather than attempting to reallocating it. This can lead to double-free problems..

    1. John McDonald

      The C standard doesn't say anything about what happens with a size argument of 0, but I checked glibc and openbsd source real quick and they appear to return a valid pointer to a zero-sized object.. e.g. the return of a malloc(0);

      I mention this because if there was a system where realloc() free'ed the object and then returned NULL, it would make the secure idiom for realloc() exploitable. Here's that idiom, snagged from an openbsd man page:

      if ((p2 = realloc(p, nsize)) == NULL) {
    if (p)
        free(p);
    p = NULL;
    return NULL;
}
p = p2;

      You can see that if nsize was 0, and realloc() free'ed the memory and then returned NULL, it would be a double-free of p. I would guess that no systems actually do this, but it might be worth researching. As it stands, the behavior with an nsize of 0 is pretty interesting and counter-intuitive, and I can see people running into trouble because of it.

      1. Robert Seacord

        OK, I've updated MEM36-C. to talk about reallocating zero bytes.

      2. Jonathan Leffler

        Solaris 10 (UltraSPARC) specifically returns NULL when you realloc 0 bytes.

        #include <stdlib.h>
#include <stdio.h>

int main(void)
{
    char *s = malloc(24);
    char *p = realloc(s, 0);
    puts((s == 0) ? "s was null" : "s was not null");
    puts((p == 0) ? "p is  null" : "p is  not null");
    return(0);
}

        This code yields "s was not null" and "p is null".

  2. Stephen Friedl

    This again points to the benefit of setting pointers to NULL after free(); this doesn't fix the logic error that calls free() twice, but it at least gets rid of the gross security issue of freeing a random pointer.

  3. Douglas A. Gwyn

    I prefer if(x!=NULL)free(x) ; since it doesn't depend on the special case for free(NULL) which some libraries might get wrong.  (Before C89, almost all of them croaked.)

    1. Jonathan Leffler

      The document assumes C99 - even if it only assumed C89, free(0) is defined as safe.  It is only pre-C89 implementations that might have the problem.  Presumably, those who work on such implementations are aware of the problems, and for the large majority who do not work on such archaic systems, there is no issue.

  4. Dhruv Mohindra

    I recall a discussion about the comments that say "handle error". Perhaps the CS would be better if it exits or returns (with an error code) after freeing the memory on error_condition = 1 rather than removing free() altogether from that part of code.

    1. David Svoboda

      I think you are referring to code that does the following:

      /* ... */
if (error_condition == 1) {
  /* handle error condition*/
  free(x);
  x = NULL;
  return; /* or transfer control, such as abort() or goto */
}
/* ... */
free(x);
x = NULL;

      This code clearly frees x just once, and so complies with the rule, but this code has the following drawbacks:

      • Its harder to specify. Just saying 'return' is invalid if the code is inside a non-void function.
      • Its easier to modify the code such that the return is deleted, which turns the code back into the NCCE.
      • The cleanup of x is duplicated inside & outside the if statement. The redundancy leads to code bloat and potential for more errors.

      I suspect these reasons are why this code sample wasn't included as a 2nd CS.

      1. Martin Sebor

        I agree that the rule would be more "believable" if the error handling was less handwavy. Maybe like so:

        int f(size_t n) {
  int error_condition = 0;

  if (n > SIZE_MAX / sizeof(int)) {
    errno = EOVERFLOW;
    return -1;
  }

  int *x = (int*)malloc(n * sizeof(int));
  if (x == NULL) {
    /* Report allocation failure to caller. */
    return -1;
  }

  /* Use x and set error_condition on error. */

  if (error_condition != 0) {
    /* Handle error condition. */
  }

  free(x);

  return error_condition;
}

        1. Dhruv Mohindra

          The above code by Martin appears to address the return problem/possible memory leak I'd imagined. +1

          1. Martin Sebor

            I've updated the code examples.

  5. David Svoboda

    MSVC error C6308 addresses an improper use of realloc(). which rule corresponds with this diagnostic? I assume its MEM31-C, but that's not obvious. Whichever rule should handle C6308 needs a NCCE & CS pair to illustrate the problem.

    1. Martin Sebor

      I wonder if MEM08-C. Use realloc() only to resize dynamically allocated arrays could be tweaked to cover the case you mention. As it stands, the problem MEM08-C tries to avoid doesn't seem like one that's specific to realloc() but rather one of converting a pointer to one object to a pointer of an unrelated type. That can just as easily happen with malloc() or in any other situation involving a cast of a pointer to another.

  6. David Svoboda

    We may want to add an exception for dynamically-loaded libraries. Most programs allocate memory to hold dynamically-loaded code (eg the LoadLibrary() function in Windows), and do nothing to free them (expecting that they will persist through the lifetime of the program.)

  7. Robert Seacord (Manager)

    This rule is going to get trashed and replaced by something that looks like the memory portions of this TS 17961 rule:

    5.18 Failing to close files or free dynamic memory when they are no longer needed [fileclose]

     

    1. John Whited

      Even when replaced, the Risk Assessment needs attention.

      Severity on this page is "Medium".  But summary on the "Memory Management" page shows Severity for this rule to be "High".  The two should align.