CWE-170: Improper Null Termination - C

Overview

C strings are just byte arrays with no built-in length - every function that treats a char * as a string (strlen, printf("%s", ...), strcmp) scans forward until it finds a \0 byte, so a buffer that is missing one, or has one in the wrong place, causes those functions to keep reading past the buffer's real end. The two most common causes are strncpy() (which does not add a terminator when the source is at least as long as the destination) and raw reads (recv(), fread()) that return exactly the bytes transferred with no terminator at all.

Common Vulnerable Patterns

strncpy without an explicit terminator

void copy_name(const char *input) {
    char buffer[10];
    // VULNERABLE - strncpy does not guarantee null termination
    strncpy(buffer, input, 10);
    // if input is 10 bytes or longer, buffer has NO terminator
    printf("Name: %s\n", buffer);   // reads past buffer looking for '\0'
}

Why this is vulnerable: strncpy() copies at most n bytes and pads with \0 only if the source is shorter than n - if the source is n bytes or longer, it fills the entire destination with copied bytes and writes no terminator at all. printf("%s", ...) then reads whatever memory follows buffer until it happens to find a zero byte.

Network and file data treated as a string

void read_from_network(int socket) {
    char buffer[256];
    int bytes_read = recv(socket, buffer, 256, 0);
    // VULNERABLE - recv() never null-terminates; buffer holds raw bytes only
    printf("Received: %s\n", buffer);
    // if 256 bytes were received, printf reads past buffer[255] looking for '\0'
}

Why this is vulnerable: recv() and fread() fill the buffer with exactly the number of bytes transferred and return that count - they have no concept of "string" and never write a terminator. Passing the raw buffer to a string function without terminating it first reads into whatever memory follows.

Missing +1 in a length-based allocation

void process_input(const char *user_input) {
    size_t len = strlen(user_input);
    // VULNERABLE - allocates exactly `len` bytes, no room for the terminator
    char *buffer = malloc(len);
    strcpy(buffer, user_input);   // writes len + 1 bytes (data + '\0') into a len-byte buffer
}

Why this is vulnerable: strlen() returns the length of the string not counting the terminator. Allocating exactly that many bytes leaves no room for the terminator strcpy() always writes, so the copy overflows the allocation by one byte - a classic off-by-one heap corruption.

A loop that overwrites the terminator

void sanitize_filename(char *str) {
    size_t len = strlen(str);
    // VULNERABLE - <= includes the terminator's position
    for (size_t i = 0; i <= len; i++) {
        if (!isalnum((unsigned char)str[i])) {
            str[i] = '_';
        }
    }
    // at i == len, '\0' is not alphanumeric, so the terminator becomes '_'
}

Why this is vulnerable: Valid character indices are 0 to len - 1; index len holds the terminator. Using <= instead of < processes that position too, and here the transform maps '\0' to '_' - so the string loses its terminator and every later strlen, printf("%s", ...) or strcmp runs on into whatever follows it in memory.

Whether the off-by-one actually does damage depends entirely on what the loop writes, which is why this shape survives review. The same i <= len bound with str[i] = toupper(str[i]) is harmless: the C standard has toupper return its argument unchanged for anything that is not a lowercase letter, so toupper('\0') is '\0' and the terminator is rewritten with itself. A reviewer who checks the bound against that version concludes the pattern is safe and carries the conclusion to this one, where the transform has no such fixed point. Read what the body writes at index len, not just the comparison in the loop header.

Secure Patterns

Explicit termination after strncpy

void copy_name_safe(const char *input) {
    char buffer[10];
    strncpy(buffer, input, sizeof(buffer) - 1);   // reserve the last byte
    buffer[sizeof(buffer) - 1] = '\0';            // always terminate explicitly
    printf("Name: %s\n", buffer);
}

Why this works: Copying at most sizeof(buffer) - 1 bytes guarantees the last byte was never touched by the copy, so setting it to '\0' afterward always lands inside the buffer. The result is a valid C string whether or not input was longer than the destination.

That final byte is a backstop, not the end of the string. When input is longer than the destination, strncpy fills all sizeof(buffer) - 1 bytes without terminating and the explicit write at index 9 is the terminator. When input is shorter, strncpy zero-pads the rest of the destination, so the string already ended earlier and the write at index 9 lands on a byte that was already '\0' - harmless, and doing nothing.

The padding is what makes the fixed offset safe here, and it is specific to strncpy. A copy that truncates without padding - memcpy with a computed length - leaves the buffer's previous contents between the copied data and index 9, so the same two lines produce a terminated string with stale bytes inside it. With a primitive like that, terminate at the number of bytes actually copied instead. It is also the reason strncpy is slower than it looks on a large destination: it writes the whole buffer every time, not just the input.

Terminating network and file reads explicitly

void read_from_network_safe(int socket) {
    char buffer[256];
    ssize_t bytes_read = recv(socket, buffer, sizeof(buffer) - 1, 0);
    if (bytes_read < 0) {
        return;                      // error - recv wrote nothing, buffer is untouched
    }
    buffer[bytes_read] = '\0';       // terminate right after the data received;
                                     // bytes_read == 0 (peer closed) gives an empty string
    printf("Received: %s\n", buffer);
}

Why this works: Reading sizeof(buffer) - 1 bytes reserves room for a terminator that the read itself will never provide, and placing it at buffer[bytes_read] puts it exactly at the end of the data actually received - not at a fixed offset that might be wrong if fewer bytes arrived than the buffer's capacity.

The guard tests < 0 rather than > 0 on purpose. recv() returns 0 when the peer has closed the connection, and a if (bytes_read > 0) guard skips the terminator write on exactly that path - so the buffer stays as unterminated as it was before the fix, on an input a reviewer is unlikely to try. Only a negative return means nothing was written; every non-negative count should be terminated.

One recv() is also not a message. It returns whatever has arrived so far, so a caller that needs a whole line or a length-prefixed frame has to loop, and the terminator belongs after the last byte of the accumulated data, not after each partial read.

Correct allocation and loop bounds

#include <ctype.h>
#include <stdlib.h>
#include <string.h>

char *process_input_safe(const char *user_input) {
    size_t len = strlen(user_input);
    char *buffer = malloc(len + 1);       // +1 for the terminator
    if (buffer == NULL) return NULL;
    memcpy(buffer, user_input, len);
    buffer[len] = '\0';
    return buffer;                        // caller owns it and must free()
}

void sanitize_filename_safe(char *str) {
    size_t len = strlen(str);
    for (size_t i = 0; i < len; i++) {    // < excludes the terminator's position
        if (!isalnum((unsigned char)str[i])) {
            str[i] = '_';
        }
    }
}

Why this works: Allocating strlen(input) + 1 bytes leaves room for the terminator - memcpy copies len bytes and writes none of its own, so the explicit buffer[len] = '\0' lands on the byte the + 1 reserved. Using < instead of <= in the loop bound keeps every write inside [0, len), so index len - the terminator - is never written, whatever the loop body would have put there.

That last clause is the point of the bound. < is correct regardless of what the body does, so it holds when someone later changes the transform; a bound that happens to be survivable because the current body maps '\0' to '\0' is a property of the body, not of the loop, and it is lost the moment the body changes.

The (unsigned char) cast on the isalnum() argument is not cosmetic either. char is signed on most platforms, so a byte from a UTF-8 or Latin-1 string arrives as a negative int - which is neither EOF nor a value the <ctype.h> functions are defined for, and is undefined behaviour rather than a no-op. The same cast belongs on every <ctype.h> call that takes a byte out of a char buffer.

Prefer functions that always terminate

#include <stdio.h>

void format_message_safe(const char *username, int score) {
    char buffer[50];
    int written = snprintf(buffer, sizeof(buffer), "User: %s, Score: %d", username, score);
    if (written < 0 || (size_t)written >= sizeof(buffer)) {
        // truncated or encoding error - buffer is still safely terminated either way
    }
}

Why this works: snprintf() writes at most size - 1 characters and always places a terminator within the buffer, even when the formatted output would have been longer - there is no code path where it produces an unterminated result. Its return value reports the length that would have been written, so truncation is detectable rather than silent.

Safe function comparison:

Function	Always Terminates?	Bounds-Checked?
strcpy	Yes, but no bounds check	No
strncpy	No (if source >= n)	Yes
snprintf	Always	Yes
strlcpy (BSD/glibc 2.38+)	Always	Yes
strcpy_s (C11 Annex K, optional)	Always	Yes

Testing

• Build with AddressSanitizer (-fsanitize=address -g -O1) and test with input exactly at, and one byte over, the buffer's capacity.
• Test recv()/fread() paths with a payload that completely fills the read buffer (no bytes left over for a terminator without an explicit - 1 reservation).
• Run under Valgrind as an independent check for the resulting out-of-bounds reads.
• Fuzz any function that parses untrusted input into a fixed buffer.

Common Pitfalls

• Increasing the buffer size instead of fixing the copy logic: making the destination bigger reduces how often a missing terminator causes visible corruption, but doesn't guarantee termination if the code path that fills it never writes the terminator byte - a large-enough input still reproduces the exact same bug.
• Trusting strncpy's name to imply safety: strncpy is bounded, but "bounded" and "terminated" are different guarantees - it only implies the second when the source is shorter than the limit.
• Forgetting the terminator when re-slicing an already-terminated buffer: copying a substring out of a valid C string with a raw memcpy of n bytes produces a buffer that is not terminated at all, even though the original string was - each new buffer needs its own terminator, it isn't inherited.

Additional Resources

• CWE-170: Improper Null Termination
• CERT STR32-C: Do not pass a non-null-terminated character sequence to a library function that expects a string
• cppreference: strncpy