SEI CERT C Coding Standard

Space shortcuts

Page tree

Dynamic memory management is a common source of programming flaws that can lead to security vulnerabilities. Poor memory management can lead to security issues, such as heap-buffer overflows, dangling pointers, and double-free issues [Seacord 2013]. From the programmer's perspective, memory management involves allocating memory, reading and writing to memory, and deallocating memory.

Allocating and freeing memory in different modules and levels of abstraction may make it difficult to determine when and if a block of memory has been freed, leading to programming defects, such as memory leaks, double-free vulnerabilities, accessing freed memory, or writing to freed or unallocated memory.

To avoid these situations, memory should be allocated and freed at the same level of abstraction and, ideally, in the same code module. This includes the use of the following memory allocation and deallocation functions described in subclause 7.23.3 of the C Standard [ISO/IEC 9899:2011]:

void *malloc(size_t size);

void *calloc(size_t nmemb, size_t size);

void *realloc(void *ptr, size_t size);

void *aligned_alloc(size_t alignment, size_t size);
 
void free(void *ptr);

Failing to follow this recommendation has led to real-world vulnerabilities. For example, freeing memory in different modules resulted in a vulnerability in MIT Kerberos 5 [MIT 2004]. The MIT Kerberos 5 code in this case contained error-handling logic, which freed memory allocated by the ASN.1 decoders if pointers to the allocated memory were non-null. However, if a detectable error occurred, the ASN.1 decoders freed the memory that they had allocated. When some library functions received errors from the ASN.1 decoders, they also attempted to free the same memory, resulting in a double-free vulnerability.

Noncompliant Code Example

This noncompliant code example shows a double-free vulnerability resulting from memory being allocated and freed at differing levels of abstraction. In this example, memory for the list array is allocated in the process_list() function. The array is then passed to the verify_size() function that performs error checking on the size of the list. If the size of the list is below a minimum size, the memory allocated to the list is freed, and the function returns to the caller. The calling function then frees this same memory again, resulting in a double-free and potentially exploitable vulnerability.

enum { MIN_SIZE_ALLOWED = 32 };

int verify_size(char *list, size_t size) {
  if (size < MIN_SIZE_ALLOWED) {
    /* Handle error condition */
    free(list);
    return -1;
  }
  return 0;
}

void process_list(size_t number) {
  char *list = (char *)malloc(number);
  if (list == NULL) {
    /* Handle allocation error */
  }

  if (verify_size(list, number) == -1) {
      free(list);
      return;
  }

  /* Continue processing list */

  free(list);
}

The call to free memory in the verify_size() function takes place in a subroutine of the process_list() function, at a different level of abstraction from the allocation, resulting in a violation of this recommendation. The memory deallocation also occurs in error-handling code, which is frequently not as well tested as "green paths" through the code.

Compliant Solution

To correct this problem, the error-handling code in verify_size() is modified so that it no longer frees list. This change ensures that list is freed only once, at the same level of abstraction, in the process_list() function.

enum { MIN_SIZE_ALLOWED = 32 };

int verify_size(const char *list, size_t size) {
  if (size < MIN_SIZE_ALLOWED) {
    /* Handle error condition */
    return -1;
  }
  return 0;
}

void process_list(size_t number) {
  char *list = (char *)malloc(number);

  if (list == NULL) {
    /* Handle allocation error */
  }

  if (verify_size(list, number) == -1) {
      free(list);
      return;
  }

  /* Continue processing list */

  free(list);
}

Risk Assessment

The mismanagement of memory can lead to freeing memory multiple times or writing to already freed memory. Both of these coding errors can result in an attacker executing arbitrary code with the permissions of the vulnerable process. Memory management errors can also lead to resource depletion and denial-of-service attacks.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

MEM00-C

High

Probable

Medium

P12

L1

Automated Detection

Tool

Version

Checker

Description

CodeSonar
8.1p0

ALLOC.DF
ALLOC.LEAK

Double free
Leak

Compass/ROSE



Could detect possible violations by reporting any function that has malloc() or free() but not both. This would catch some false positives, as there would be no way to tell if malloc() and free() are at the same level of abstraction if they are in different functions

Coverity6.5RESOURCE_LEAKFully implemented
Klocwork
2024.1
FREE.INCONSISTENT
UFM.FFM.MIGHT
UFM.FFM.MUST
UFM.DEREF.MIGHT
UFM.DEREF.MUST
UFM.RETURN.MIGHT
UFM.RETURN.MUST
UFM.USE.MIGHT
UFM.USE.MUST
MLK.MIGHT
MLK.MUST
MLK.RET.MIGHT
MLK.RET.MUST
FNH.MIGHT
FNH.MUST
FUM.GEN.MIGHT
FUM.GEN.MUST
RH.LEAK
LDRA tool suite
9.7.1

50 D

Partially implemented

Parasoft C/C++test
2023.1

CERT_C-MEM00-a
CERT_C-MEM00-b
CERT_C-MEM00-c
CERT_C-MEM00-d
CERT_C-MEM00-e

Do not allocate memory and expect that someone else will deallocate it later
Do not allocate memory and expect that someone else will deallocate it later
Do not allocate memory and expect that someone else will deallocate it later
Do not use resources that have been freed
Ensure resources are freed

Parasoft Insure++

Runtime analysis
PC-lint Plus

1.4

449, 2434

Partially supported

Polyspace Bug Finder

R2023b

CERT C: Rec. MEM00-C


Checks for:

  • Invalid free of pointer
  • Deallocation of previously deallocated pointer
  • Use of previously freed pointer

Rec. partially covered.

Related Vulnerabilities

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

Related Guidelines

SEI CERT C++ Coding StandardVOID MEM11-CPP. Allocate and free memory in the same module, at the same level of abstraction
ISO/IEC TR 24772:2013Memory Leak [XYL]
MITRE CWECWE-415, Double free
CWE-416, Use after free

Bibliography

[MIT 2004]
[Plakosh 2005]
[Seacord 2013]Chapter 4, "Dynamic Memory Management"


21 Comments

  1. Robert Seacord

    originally posted by by pete filandro at Aug 01, 2007 21:08

    One method of allocation recommended by the rabble on news:comp.lang.c is :

    #include <stdlib.h>

    ptr = malloc(NMEMB * sizeof *ptr);

    That, plus the related expression, (ptr = malloc(sizeof *ptr)), and also:

    ptr = malloc(string_length + 1);

    can take care of most malloc call situations.

  2. Dave Aronson

    The narratives seem to assume that "Handle Error" does not include "return".  If that also applies to "Handle Allocation Error", then both examples (NCCE and CS) are vulnerable to trying to free NULL.  A quick check of "man 3 free" says that nothing happens, but I could swear I've seen that cause a crash.  Maybe that's implementation-defined, in which case it's still worth a mention.

    1. Jonathan Leffler

      Some pre-C89 compilers (strictly, the libraries) objected to freeing null pointers (usually by crashing).

      All compilers/libraries compliant with C89 or later accept free(0) as a no-op.

  3. David Svoboda

    Pretty close. One point of information is that calling free on a null pointer is perfectly kosher; it does a noop.

    The first /* Handle Allocation Error */, which operates if malloc() fails is therefore free to do anything, including return, goto, longjmp, exit() ..., or none of these. If it does not exit, the free operates on a null pointer, which does nothing.

    The second /* Handle Error /, as well as the / Continue Processing */ are both constrained from any non-local exits. So one cannot do return, goto, longjmp, exit()..., without leaking the allocated memory.

  4. David Svoboda

    The realpath() function (defined in stdlib.h) takes a pathname and returns a canonicalized pathname. The point here is that it can (depending on the parameters) return its own malloc'ed result string, which must then be freed by the caller. Many other standard library functions that must return a new string have the option to allocate it and expect the user to free the result string when done with it. Doesn't this design pattern violate this recommendation?

    C++ can sidestep this issue with RAII. A function can return an object that malloc's a string upon creation and free's it upon destruction.

    I vaguely remember reading somewhere that the main reason Java implemented garbage collection was this particular problem; how do you handle malloc'ed data returned by a library function? and how do you know WHEN you are responsible for free()ing data returned by a library function?

    1. Douglas A. Gwyn

      There should be no realpath() function declaration in <stdlib.h>.

      1. Jonathan Leffler

        Under Standard C, there would be no realpath() in <stdlib.h>.  Under POSIX, there would.

    2. Jonathan Leffler

      Doesn't this design pattern violate this recommendation?

      Yes, but as long as the responsibility for which function is responsible for freeing the allocated space is properly documented and then acted on, it is legitimate. I would suspect that one reason this is a recommendation rather than a rule is issues such as this.

      Also, according to the POSIX description of realpath():

      • If resolved_name is a null pointer, the behavior of realpath() is implementation-defined.

      I take it that the implementation you are familiar with documents the implementation-defined behaviour as "realpath() will allocate enough space for the path via malloc() and the caller is responsible for freeing the allocated space".

  5. Douglas A. Gwyn

    This rule cannot be followed when dynamic allocation is used for linked data structures, which is probably its most important application.  A better rule is "have only one node freeing function and ensure that it maintains the integriry of the data structure."  It is also wise to use "handles" (volatile pointer to pointer to allocated chunk) that get NULLified by the freeing function.  Generally speaking, a lot of thought should go into the design.

    1. Robert Seacord

      Why volatile?

  6. Alex Volkovitsky

    I disagree with the ROSE suggestion for this rule, what if a program has wrappers for malloc() and free()? The wrappers may be called within the same function, but each wrapper only calls malloc() OR free() accordingly.

    1. David Svoboda

      Alex and I discussed this today. I have removed the 'rose-possible' tag, but am not convinced that writing a rose rule is worthless...what do others think? Here is the conclusion that Alex and I came to:

      The tool Compass / ROSE does not currently detect violations of this recommendation, but it could. One merely has to search a function's local block for a call to either malloc() or free(), but not both. A function could have multiple calls to free() for each malloc() call, as the compliant solution illustrates. This would catch cases such as the non-compliant example above. However, there is a common coding style where malloc() is called inside an init function, and free() is called inside a destroy function, which mimicks C++ constructors and destructors. ROSE would flag these as false positives, and there is no good heuristic to train ROSE to identify and accept these patterns.

      1. David Svoboda

        Currently marked as rose-nonapplicable. How do LDRA and Fortify catch violations? How do they avoid false positives on malloc in an init function or free in a destroy function?

        This might be a good argument to have ROSE flag such rules, but only if in a 'sensitive' mode where false positives are less severe.

        1. David Svoboda

          Added back rose algo section, marked as 'rose-false-positive'.

  7. Martin Sebor

    The advice here should apply equally well to other kinds of resources besides memory, including file pointers and descriptors, threads, processes, and synchronization primitives, etc. Would it make sense to generalize this guideline to encompass all types of resources or should there be a separate guideline for resources other than memory?

    1. David Svoboda

      Agreed. (I think we do have a rule about closing open files, in the FIO section somewhere.)

  8. Dhruv Mohindra

    verify_list() in the description should probably be verify_size().

    1. David Svoboda

      fixed

  9. Yozo TODA

    I removed a sentence (which looks to me redundant) from the beginning paragraph.

    Revert to the previous version if it lose the original intended meaning...

  10. Joseph C. Sible

    Wouldn't this rule ban functions like strdup, getline, and asprintf? In particular, isn't it often best for the callee to allocate its own memory, since the caller can't know how much will be needed?

    1. David Svoboda

      Strictly as written, this recommendation does not account for functions like strdup() which returns a block of memory that must be subsequently freed. This is a recommendation, not a rule, and this means you can violate it while still having secure coding.

      We could always add an exception allowing functions to produce allocated chunks like malloc() does, or to consume them like free() does. There are lots of other exceptions we would have to excuse, such as smart pointers, garbage collection, and other techniques.

      More sophisticated techniques for modelling memory allocation exist, too. I'll recommend the Pointer Ownership Model (https://resources.sei.cmu.edu/library/asset-view.cfm?assetid=55000) as an example.