misc/124410: malloc exposes previously free'd memory

[Rene Schickbauer]

>Number:         124410
>Category:       misc
>Synopsis:       malloc exposes previously free'd memory
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Jun 09 08:40:01 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Rene Schickbauer
>Release:        7.0-stable
>Organization:
Magna Powertrain
>Environment:
FreeBSD magic3.magicbooks.org 7.0-STABLE FreeBSD 7.0-STABLE #1: Thu Mar 20 15:43:26 CET 2008     root at magic3.magicbooks.org:/usr/obj/usr/src/sys/MAGIC3  i386

>Description:
malloc() exposes memory content from previous malloc/memory operations/free cycles.

This can pose a security risk under certain circumstances. For example, a program generates a key, and forgets to memset() before it frees the memory. The memory contents (and the key) *may* be exposed on subsequent library calls or through unsafe network operations. E.g. this may expose security related data within a running process even after the process has "disposed" all of that data.

While the stand-alone risk may (or may not) be very low, this security leak may work in conjuction with other software bugs to create a deep security hole.

This especially the case, as many software developers seem to think that a free() disposes of the data.
>How-To-Repeat:
Source code of test program:

[cavac at magic3 ~/src/memtest]$ cat main.cpp
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#define NUM_SEGMENTS 10
#define SIZE_SEGMENTS 1000

int main() {
    FILE *fh;
    printf("Opening file...\n");
    fh = fopen("test.dat", "wb");
    if(!fh) {
        fprintf(stderr, "Can't open file for writing!\n");
        exit(1);
    }

    // Test malloc'ing multiple segments
    printf("Test 1: Multiple segments\n");
    void *mem[10];
    for(int i=0; i < NUM_SEGMENTS; i++) {
        printf("  %d / %d\n", i+1, NUM_SEGMENTS);
        void *xmem = malloc(SIZE_SEGMENTS);
        if(!xmem) {
            fprintf(stderr, "Out of memory (or other malloc problem)!\n");
            exit(2);
        }
        fwrite(xmem, SIZE_SEGMENTS, 1, fh);
        mem[i] = xmem;
    }
    //  Free mem again
    for(int i=0; i < NUM_SEGMENTS; i++) {
        free(mem[i]);
    }

    // Test malloc'ing multiple segments
    printf("Test 2: malloc/free cycle with memset(mem,1,SIZE_SEGMENT)\nbefore free\n");
    for(int i=0; i < NUM_SEGMENTS; i++) {
        printf("  %d / %d\n", i+1, NUM_SEGMENTS);
        void *mem = malloc(SIZE_SEGMENTS);
        if(!mem) {
            fprintf(stderr, "Out of memory (or other malloc problem)!\n");
            exit(2);
        }
        fwrite(mem, SIZE_SEGMENTS, 1, fh);
        memset(mem,1,SIZE_SEGMENTS);
        free(mem);
    }


    printf("Closing file...\n");
    fclose(fh);
    printf("DONE\n");
}


Compile with: 
g++ -o memtest main.cpp

And run as:
./memtest

This creates a file "test.dat", which holds all data that we got in "malloc()"

Then run "hexdump":
[cavac at magic3 ~/src/memtest]$ hexdump -C test.dat
00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00002af0  00 00 00 00 00 00 00 00  01 01 01 01 01 01 01 01  |................|
00002b00  01 01 01 01 01 01 01 01  01 01 01 01 01 01 01 01  |................|
*
00004e20

As shown by hexdump, it gives cleared memory on first malloc (test 1), but holds data (in test 2) previously written before free().
>Fix:
The safest fix would be to change free() to memset() the allocated memory before freeing it. Also, malloc() should memset() in all cases.

As this will decrease system performance, it should be configureable.

>Release-Note:
>Audit-Trail:
>Unformatted: