Skip to content

CWE-477: Use of Obsolete Function - C

Overview

C's standard library carries functions that the standard or POSIX has since withdrawn or superseded: gets(), removed from the C standard in C11; tmpnam() and tempnam(), which have a race condition baked into the API; and getwd(), bcopy() and bzero(), dropped from POSIX.1-2008. Most compilers still accept them for compatibility with existing code, so they keep compiling and running long after the standard stopped listing them.

strcpy(), strcat() and sprintf() are a different case and are covered on the CWE-676 C page. They are current, standard C with a documented safe calling convention - dangerous to call carelessly, but not obsolete - and a scanner that files them under CWE-477 has the wrong number.

rand() and srand() are the same kind of mismatch and are covered below anyway, because scanners report them here constantly. No standard has deprecated or removed either: they are current C and correct for everything that does not need unpredictability. Using one for a token or a session identifier is CWE-338 - a weak PRNG chosen for a security purpose - and unlike the removals above, whether it is a finding at all depends on what predicting the value would give an attacker.

Primary Defence: Replace gets() with fgets() and tmpnam()/tempnam() with mkstemp(). For rand(), first decide whether the value has to be unpredictable; where it does, draw it from a cryptographically secure source instead.

Common Vulnerable Patterns

Functions the standard withdrew

// VULNERABLE - gets() cannot be bounded and was removed from C11
char buf[10];
gets(buf);                 // no size argument exists - overflow on any line over 9 chars

// VULNERABLE - removed from POSIX.1-2008
char cwd[256];
getwd(cwd);                // no size argument either; getcwd(cwd, sizeof(cwd)) replaced it
bcopy(src, dst, n);        // replaced by memmove(dst, src, n) - note the argument order
bzero(buf, n);             // replaced by memset(buf, 0, n)

Why this is vulnerable: gets() and getwd() share the defect that got them withdrawn - neither takes the destination buffer's size, so neither can stop at its end no matter how the caller writes the call. That is what separates them from strcpy(), whose signature is equally silent but whose length the caller can at least check beforehand: there is no gets() call site that a preceding check makes safe, because the length is not known until after the write has happened.

bcopy() and bzero() are a milder case - they are not unsafe, they are simply gone, so code using them fails to build against a current libc header set or silently picks up a compatibility shim. The hazard is in the migration rather than the original call: bcopy takes source first and handles overlapping regions, so the natural-looking swap to memcpy reverses the meaning of two arguments and drops the overlap guarantee. memmove is the correct target.

Predictable Random Values for Security Purposes (reported here, but CWE-338)

// VULNERABLE - predictable, not cryptographically secure
srand(time(NULL));           // seed is guessable - it's roughly "now"
int token = rand();          // output is a predictable PRNG sequence
int sessionId = rand() % 1000000;

Why this is vulnerable: rand() is a standard pseudo-random generator with no security guarantees - given a few outputs, an attacker can often recover the internal state and predict future values. Seeding with time(NULL) narrows the search space further, since the seed is close to a known value (the time the process started).

rand() has not been withdrawn and calling it is not the defect; using its output where the value must be unguessable is. That makes this the one pattern on the page whose fix needs a judgement rather than a substitution - rand() shuffling a demo deck stays, rand() minting a session identifier goes - and it is why the finding properly belongs to CWE-338. It is documented here because that is where scanners file it.

Race-Prone Temporary File Creation

// VULNERABLE - predictable name, race condition between name and open
char *filename = tmpnam(NULL);
FILE *f = fopen(filename, "w");  // another process can create/symlink this path first

char *filename2 = tempnam("/tmp", "prefix");  // same problem

Why this is vulnerable: tmpnam() and tempnam() only generate a name - they don't create the file. Between the name being generated and fopen() creating it, another process can create a file (or a symlink) at that path, letting an attacker redirect the write or read data intended to be private.

Secure Patterns

Bounds-Checked String Handling

#include <stdio.h>
#include <string.h>

void read_and_copy(const char *input, char *dest, size_t dest_size) {
    char buf[10];

    // bounded input, the replacement for gets(): fgets keeps the newline and
    // always terminates, and NULL means end-of-file or error rather than empty input
    if (fgets(buf, sizeof(buf), stdin) != NULL) {
        buf[strcspn(buf, "\n")] = '\0';   // strip the newline fgets retains
    }

    // bounded formatted output - always terminates, reports truncation via its return
    snprintf(buf, sizeof(buf), "%s", input);

    // strlcpy/strlcat null-terminate on every path (BSD, macOS, and glibc 2.38+)
    strlcpy(dest, input, dest_size);
    strlcat(dest, buf, dest_size);

    // strncpy is bounded but NOT guaranteed to terminate - the explicit write is required
    strncpy(buf, input, sizeof(buf) - 1);
    buf[sizeof(buf) - 1] = '\0';
}

Why this works: Every replacement takes the destination buffer's size and stops writing at that limit, so an oversized input gets truncated instead of overflowing the buffer. fgets() is the direct successor to gets() - same job, plus the size argument gets() had no way to accept - and it is the only one of these that also tells the caller, via a NULL return, that nothing was read.

strncpy is listed last on purpose. It is bounded but not terminating: whenever the source is as long as or longer than its size argument it fills the destination and writes no terminator, which here means any input of sizeof(buf) - 1 bytes or more, not just one that fills the buffer exactly. That is CWE-170, and mechanically swapping strcpy for strncpy trades a buffer overflow for an unterminated string while satisfying the scanner. snprintf and strlcpy terminate on every path and are the better default; strlcpy and strlcat are in glibc from 2.38 onwards as well as on the BSDs and macOS, so the portability argument for strncpy is weaker than it used to be.

Cryptographically Secure Random Values

#include <limits.h>
#include <openssl/rand.h>

int make_token(unsigned char *token, size_t len) {
    if (len > INT_MAX) {
        return -1;   // RAND_bytes takes an int; narrowing a larger size_t here would
                     // silently ask for fewer bytes and still report success
    }
    if (RAND_bytes(token, (int)len) != 1) {
        return -1;   // RAND_bytes can fail if the entropy source is unavailable;
                     // token holds nothing usable, so do not carry on
    }
    return 0;
}

Without OpenSSL, ask the kernel directly rather than opening /dev/urandom by hand:

#include <sys/random.h>   // Linux glibc 2.25+; on BSD/macOS use arc4random_buf()

int make_token(unsigned char *token, size_t len) {
    ssize_t n = getrandom(token, len, 0);
    if (n < 0) {
        return -1;                  // error - nothing usable was written
    }
    if ((size_t)n != len) {
        return -1;                  // short count - the buffer is not fully random
    }
    return 0;
}

Why this works: RAND_bytes() and getrandom() both draw from a cryptographically secure source designed so that past output reveals nothing about future output: unlike rand(), they do not let an attacker who has seen some of their output predict what comes next.

Both of these can fail, and the failure is the part worth writing carefully. RAND_bytes() returns 0 or -1 when the entropy source is unavailable, and getrandom() returns -1 on error - including EAGAIN when GRND_NONBLOCK is set and the urandom pool is not yet initialised - and a count smaller than requested when a read of more than 256 bytes is interrupted by a signal, or when GRND_RANDOM is set and the random source has fewer bytes available than asked for. In every one of those cases token keeps whatever was on the stack, which is often the previous caller's data and is at best not random. A call whose return value is ignored produces a token that looks fine in a debugger and is not secret, which is worse than the rand() it replaced because nothing about the code suggests a problem.

Test the error before comparing the counts, and keep the comparison in one type. Folding both into getrandom(...) != (ssize_t)len looks tighter and has a hole at the end of the range: (ssize_t)len is -1 when len is SIZE_MAX, so a failing call returning -1 compares equal to the expected length and the helper reports success having written nothing. Checking n < 0 first, then comparing (size_t)n against len, has no such case - by the second test n is known non-negative, so the cast is exact either way.

The same applies to the size argument on the OpenSSL side, and it is the more likely of the two to be reached. RAND_bytes() takes an int while the helper's own parameter is a size_t, so a request above INT_MAX narrows on the way in: the call fills the truncated count, returns 1, and the helper reports success for a buffer that is mostly untouched. A cast written to silence a conversion warning is exactly how that arrives - bound the value instead, and let an oversized request fail loudly.

arc4random_buf() on BSD and macOS is the exception to all of this: it cannot fail, takes a size_t, and returns void, so there is nothing to check.

Opening /dev/urandom yourself is a third option and the one to avoid where a syscall exists. It needs a file descriptor available (it fails inside a restrictive chroot or an exhausted fd table), and read() on it may return fewer bytes than asked for, so a correct version is a loop with two error paths rather than the three unchecked lines it is usually written as.

Atomic, Unpredictable Temporary Files

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int write_scratch_file(const char *data) {
    char template[] = "/tmp/myapp.XXXXXX";
    int fd = mkstemp(template);      // creates the file atomically, mode 0600
    if (fd == -1) {
        perror("mkstemp");
        return -1;                   // must return - fdopen(-1, ...) below is not a fallback
    }

    FILE *f = fdopen(fd, "w");
    if (f == NULL) {
        close(fd);
        unlink(template);
        return -1;
    }

    fputs(data, f);
    fclose(f);                       // also closes fd
    unlink(template);                // remove when done, if the file shouldn't persist
    return 0;
}

Why this works: mkstemp() creates the file atomically as part of generating the name, closing the window where another process could race to create or symlink the same path first. The filename is unpredictable (the XXXXXX is replaced with random characters), and the file is created with mode 0600 by default - readable and writable only by its owner.

Note the return in the error branch. mkstemp() returning -1 means no file was created and template was overwritten with an unusable name; falling through to fdopen(-1, "w") is not a degraded mode, it is a null FILE * that the next fprintf dereferences. This is the shape a bounds fix most often leaves behind - the error is detected and reported, and then execution continues as if it had not happened.

mkstemp also relies on the directory it writes into. /tmp is world-writable, so mode 0600 and an unpredictable name are what protect the file, not the location; where the data is sensitive, prefer a directory the application owns, or create one with mkdtemp() and put the file inside it.

Considerations

rand() is only a finding when the value has to be unpredictable. Jitter on a retry backoff, a shuffled demo dataset, a sampling decision in a metrics pipeline - none of these give an attacker anything when guessed, and rewriting them to getrandom() adds a syscall and an error path for nothing. The question to ask is what someone gets by predicting the number: if the answer is a session identifier, a password reset code, a filename another process could pre-create, or a key, it is real. Record the false positives with that reasoning next to the call, because the next reader will otherwise re-open the same question.

tmpnam() and tempnam() are findings regardless of what the file holds. The weakness is the gap between generating the name and creating the file, and an attacker who wins that race controls where the write lands - the sensitivity of the content decides how bad the outcome is, not whether there is one.

Check what the platform actually ships before planning the substitution. strlcpy needs glibc 2.38 or a BSD; getrandom() needs Linux 3.17 with glibc 2.25 or later, and does not exist on macOS, where arc4random_buf() is the equivalent. On a codebase that must build against an older or mixed target, the portable answer is snprintf for strings and OpenSSL's RAND_bytes() for randomness rather than a per-platform #ifdef ladder.

Testing

  • Compile with AddressSanitizer (-fsanitize=address) and fuzz string-handling functions with inputs at, below, and above the buffer size.
  • For replaced random generation, assert the call sites rather than the output: that every RAND_bytes()/getrandom() return value is checked, and that a forced failure (an LD_PRELOAD stub returning 0, or getrandom() returning a short count) aborts the operation instead of emitting a token. Statistical test suites such as dieharder cannot do this job - rand() passes most of their tests, because what separates a CSPRNG from a good general-purpose PRNG is state recovery, not distribution.
  • Verify mkstemp()-created files have mode 0600 and that concurrent processes can't predict the filename.
  • Verify the error branch of every replacement returns rather than falling through: call mkstemp() against an unwritable directory and confirm the function reports failure instead of proceeding with fd == -1.

Additional Resources