CWE-252 describes a product that does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
It looks like a minor style issue. It is not. Several of the observed CVEs below are chains where this one omission opened the way to a larger flaw.
How it happens
MITRE names two assumptions behind it: "this function call can never fail" and "it doesn't matter if this function call fails". If an attacker can force the function to fail or to return something unexpected, the code that follows runs in a state the author never planned for.
MITRE's own example for the worst case is a program that calls a function to drop privileges but never checks the return code. If the drop failed, the program keeps working with the higher privileges.
/* Vulnerable: if setuid fails, we are still root */
setuid(1001);
run_untrusted_work();
/* Fixed: stop if privileges were not dropped */
if (setuid(1001) != 0) {
perror("setuid");
exit(1);
}
run_untrusted_work();
Three more patterns from MITRE's examples.
fgets without a check. The code calls fgets() and then strcpy() on the buffer. If an I/O error occurs, fgets() does not null terminate the buffer. If end of file comes before any characters are read, it writes nothing. In both cases it returns NULL to signal the problem. Ignoring that leaves an unterminated buffer, and the later strcpy() can overflow.
A size computed from an error code. A helper returns a chunk size, or minus one to indicate an error. The caller subtracts one from the result and passes it to memcpy() without checking. The error value ends up as the size argument. memcpy() treats sizes as unsigned, so minus one becomes a huge number and far more memory is copied than the destination can hold.
malloc without a check. The code uses the pointer from malloc() immediately. The usual defense is that if the program runs out of memory it will fail anyway, so it does not matter whether it fails cleanly. MITRE disputes this. Handling the error may let you free memory that is held elsewhere and carry on. It lets the program exit gracefully when required. It avoids leaving an atomic operation half done. And it lets you record diagnostic information about why the allocation failed.
buf = malloc(req_size);
if (buf == NULL) {
log_error("allocation of %zu bytes failed", req_size);
return ERR_NOMEM;
}
What an attacker gains
MITRE lists availability and integrity: an unexpected state, and a crash, exit or restart. An unexpected return value can leave the system in a state that causes a crash or other unintended behavior.
The observed examples show how much more than a crash is possible:
- CVE-2020-17533: a chain where the unchecked return value of some policy enforcement functions leads to an authorization bypass.
- CVE-2020-6078: a chain where an unchecked pointer returning function leads to use of an uninitialized variable and a null pointer dereference.
- CVE-2019-15900: a chain where an sscanf() call used to check whether a username and group exist has its return value ignored, so an uninitialized variable gets checked.
- CVE-2007-3798: an unchecked return value that leads to an integer overflow and code execution.
How to find it
MITRE lists automated static analysis, with high effectiveness. Static analysis can model the data flow and flag calls whose results are discarded.
Compiler help is cheap. Many compilers can warn when a result is ignored, and some let you mark functions so that ignoring the result is an error. Turn those on and treat the warnings as build failures for security sensitive code.
A manual pass works too. Search for calls to functions that can fail, such as privilege changes, allocation, file and socket operations, parsing and any call into a policy or permission check, and look at the next line.
If the unchecked call is inside a third party library, you will not see it in your own scan. Knowing which libraries you ship and which have published CVEs of this type helps. The free dependency scan at gold.safeguard.sh/scan runs in the browser with no login and keeps your file on your machine, up to 100 dependencies.
How to fix it
The mitigations in the record, all in the Implementation phase:
- Check the results of all functions that return a value and verify that the value is expected. MITRE rates this high effectiveness, and adds a warning to beware of race conditions in concurrent code.
- For pointers that could have been modified or that come from a function that can return NULL, check for NULL before use. In multithreaded or asynchronous code, use proper locking so the check and the use are protected together.
- Account for all possible return values. A function that returns zero, positive and negative values may mean different things in each case.
- When you design a function, return a value or throw an exception in case of error, so callers have something to check.
A habit that helps: decide the failure path before writing the success path.
Try it yourself
See how a static scan reports an ignored return value in a short sample.
Click through it at your own pace, or open it full screen.
Quick check
Why check malloc if the program would crash anyway?
Handling the failure lets you recover memory, exit gracefully, avoid leaving an operation half done and record why it failed.
What is the most serious pattern MITRE describes?
Not checking whether a privilege drop succeeded, so the program carries on with higher privileges.
Which detection method does MITRE list?
Automated static analysis, with high effectiveness.
