CWE-253 Incorrect Check of Function Return Value: Checking the Wrong Thing

CWE-253 is a close cousin of the unchecked return value weakness, with a twist. Here the code does check the result. It just checks it wrongly, which prevents it from detecting errors or exceptional conditions.

MITRE's framing is plain: important and common functions return a value that says whether their action succeeded. That value tells the program whether to handle an error. If the check is wrong, the error is never handled, and a reviewer skimming the code sees an "if" and moves on.

The weakness is listed as not language specific and not technology specific.

How it happens

Return conventions differ between functions, and they are easy to mix up. Some return zero on success. Some return zero on failure. Some return a negative number on error, and zero is a valid result. Some return a count, with a special value for "nothing" and another for "error".

A developer who guesses the convention, or copies a check from a function that behaves differently, writes a test that passes in the happy path and silently ignores the failure path.

Here are examples in C. Each follows a rule worth checking against the documentation of the function you actually call.

Treating zero as the error value for read(). The read() call returns the number of bytes read, zero at end of file, and minus one on error.

/* Vulnerable: -1 (an error) is treated as success */
ssize_t n = read(fd, buf, sizeof(buf));
if (n == 0) {
    return ERR_READ;      /* zero is end of file, not the error */
}
process(buf, n);          /* n may be -1 here */

/* Fixed: test for the real error value, handle end of file separately */
ssize_t n = read(fd, buf, sizeof(buf));
if (n < 0) {
    perror("read");
    return ERR_READ;
}
if (n == 0) {
    return END_OF_INPUT;
}
process(buf, (size_t)n);

Using a truthiness test where the function returns a count or an error code. sscanf() returns the number of items assigned, or EOF on input failure. EOF is nonzero, so a bare if (sscanf(...)) accepts the failure case as success.

/* Vulnerable: EOF is nonzero, so a failed read passes this test */
if (sscanf(line, "%d", &value)) {
    use(value);
}

/* Fixed: compare against the number of items you expect */
if (sscanf(line, "%d", &value) == 1) {
    use(value);
}

The same family includes comparing a success code to the wrong constant, reading a signed result as unsigned, and checking a pointer instead of the status that was returned next to it.

What an attacker gains

The record lists availability and integrity: an unexpected state, and a crash, exit or restart. An unexpected return value can place the system in a state that leads to a crash or other unintended behavior.

MITRE's observed example is CVE-2023-49286. It is a chain: a function in a web caching proxy does not correctly check a return value, which leads to a reachable assertion. An assertion that an outsider can trigger gives that outsider a way to stop the process.

When the vulnerable code sits in a library rather than in your own source, the practical question is whether you ship the affected version. A free dependency scan at gold.safeguard.sh/scan needs no login, keeps the file on your machine and covers up to 100 dependencies. For packages that do appear in a report, reachability analysis tells you whether your own code calls the vulnerable function, which helps you rank the work.

How to find it

MITRE lists automated static analysis. It can find some instances by modeling data flow and control flow and looking for patterns that connect sources and sinks.

Other checks:

• Read the documentation of each function you call and write down its success and failure values. Then compare them with the check in the code.
• In code review, ask one question about every status check: what does this function return when it fails?
• Add tests that force the failure, using a closed file descriptor, a full disk, a bad input or a mock that returns the error value.
• Treat compiler warnings about comparisons of signed and unsigned values, or about conditions that are always true, as findings and not noise.

How to fix it

MITRE gives two mitigations in the Implementation phase and one in Architecture and Design.

1. Language selection: use a language or compiler that uses exceptions and requires the catching of those exceptions. When failure cannot be ignored by default, there is far less to get wrong.
2. Properly check all functions that return a value.
3. When designing any function, make sure it returns a value or throws an exception in case of an error, so that callers have a clear signal.

Some habits that make these steps easier to keep:

• Wrap awkward APIs once. A thin function that converts a confusing return convention into a clear success or failure result gives every caller one correct way to test.
• Use named constants for results instead of bare numbers.
• Prefer functions that return a status separately from the data, so the two cannot be confused.
• Write the error check first, and test the error branch as carefully as the normal one.

Quick check

Sources

• MITRE CWE-253