Risk Assessment Summary
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
| MEM30-C | High | Likely | Medium | P18 | L1 |
| MEM31-C | Medium | Probable | Medium | P8 | L2 |
| MEM33-C | Low | Unlikely | Low | P3 | L3 |
| MEM34-C | High | Likely | Medium | P18 | L1 |
| MEM35-C | High | Probable | High | P6 | L2 |
| MEM36-C | Low | Probable | High | P2 | L3 |
Related Rules and Recommendations
10 Comments
Robert Seacord
I think we should add a recommendation here concerning clearing sensitive information (such as passwords) stored in dynamic memory before calling free() because the memory can be recycled leading to an unintentional disclosure. Also, it may not be possible to call realloc() on dynamic memory containing senstive information because realloc may allocate storage elsewhere and copy the sensitive data but not clear the original memory which is then also recycled.
This recommendation should also reference: http://samate.nist.gov/docs/SAMATE_source_code_analysis_tool_spec_09_15_06.pdf
However, the rule as recorded there only references realloc() and as a result is incomplete/misleading.
Dave Aronson
For some reason, my copy of Adobe's reader refuses to open that PDF. The solution seems pretty clear to me: don't call realloc, just malloc/calloc/whatever, copy the data yourself, and clear the original.
On the other claw, if we start down the road of "how to protect sensitive data", there will probably need to be LOTS of additions to this site.
Robert Seacord
Dave, That was old comment (from last year around this time). I believe MEM03-A was added to address this concern.
Douglas A. Gwyn
realloc was created to handle large allocations, where unnecessary copying takes too much time and also where there might not be room for a copy.
As to clearing data before realloc/free, we don't clear data on the stack when returning from a function. Generally neither is a problem unless there is a bug in the code.
Robert Seacord
I think we should go through some of the rules and recommendations in this section and talk about how they can be mitigated through the use of tools such as valgrind and purify, possibly others.
kowsik
Most programmer's tend to use malloc/free for variable length arrays within functions. This is a bit of an overkill and typically leads to memory leaks and what not. Either use alloca or the GCC extension for the variable array initializers.This goes something like this:
void
foo(uint32_t nelements)
{
uint32_t array[nelements];
...
}
Internally this uses alloca and is pretty safe and you have no memory leaks whatsoever. I do have to say that usage of this is when you know that the nelements is not controlled by someone outside the program. This obviously can lead to a stack overflow if the attacker controls this. It's a trade off, nevertheless.
Greg Beeley
There is actually a recommendation above, MEM05-A, which advises against the use of large stack allocations and mentions that alloca() should be avoided. I concur with the recommendation against large stack allocations, though I have not reviewed the reference on alloca() (but I don't use it myself). As far as I can tell, these recommendations do avoid dependencies on gcc extensions.
Douglas A. Gwyn
Never use alloca. (See my comments in the Gnu source for alloca.) Under C99, you can use VLAs. Cince they require a FIFO lifetime model, neither is as general as dynamic allocation.
Jonathan Day
There probably needs to be a recommendation on how to deal with security models (such as mandatory access controls) with regards memory management. I would suggest extending the recommendation of not writing sensitive data to disk (MEM06-A) to the more general recommendation of using the appropriate set of security methods for sensitive data, where pagelocking (to prevent swapping to disk) and other methods of keeping the data in memory are the core security methods that apply to all sensitive data. Other methods - such as not allocating from shared memory, using system-supplied access controls, etc - would then be extensions to this according to some specified criteria.
I'm also going to suggest a recommendation on transparency. Do programmers/maintainers really need to know if the memory is allocated by the system or the application? Whether it is truly dynamic, from a pre-allocated pool, or contained within a wholly static object? Where programmers do not need to know how or where memory is obtained, merely that it is and that it's the correct size, then the existing requirements and recommendations can be largely enforced and the variability be made safe by prohibiting the direct use of system memory management functions except within an abstraction layer.
Greg Beeley
I looked over the site and couldn't find one, but I think there should be a rule or recommendation: Do not store direct pointers to elements of a dynamic array (or string) that is subject to resizing via the realloc() function. Use offsets/indices instead. If the array is reallocated, it might be moved to a different location in memory, invalidating those pointers and causing program unreliability (some things might continue to coincidentally work until the original memory block is reused).
This can become especially tricky when a dynamic array of structures is involved (as opposed to a dynamic array of pointers to structures).