Critical Memory Safety Vulnerabilities in FITS File Processing: Buffer Overflows, Integer Overflows, and Unsafe String Operations Fixed

Introduction

If you've ever worked with astronomical software or scientific data processing pipelines written in C, you know that parsing binary file formats is one of the most dangerous operations a program can perform. FITS (Flexible Image Transport System) files — the standard format for astronomical image data — are no exception. They contain rich metadata headers, World Coordinate System (WCS) parameters, and large binary data sections, all of which must be carefully parsed.

A recent audit of src/try_to_guess_image_fov.c, a C module responsible for estimating the field of view (FOV) of astronomical images, uncovered a critical-severity cluster of memory safety vulnerabilities. These issues — classified under CWE-120 (Buffer Copy Without Checking Size of Input) — could allow an attacker who controls a FITS file to trigger buffer overflows, use-after-free conditions, and integer overflows, ultimately leading to arbitrary code execution.

This post breaks down exactly what went wrong, how the vulnerabilities could be exploited, and what the fix looks like in practice.

The Vulnerability Explained

What Is CWE-120?

CWE-120, "Buffer Copy Without Checking Size of Input," describes a class of vulnerabilities where a program copies data into a fixed-size buffer without adequately verifying that the input fits within the allocated space. In C, this is a perennial problem because the language gives developers direct memory access with minimal guardrails.

The vulnerabilities in try_to_guess_image_fov.c weren't a single bug — they were a constellation of unsafe patterns that, taken together, created a highly exploitable attack surface:

1. Unsafe strncpy usage with environment variable input
2. malloc without zero-initialization for pointer arrays
3. Redundant sizeof(char) multiplications masking integer overflow risks
4. Unsafe strncpy for command-line argument handling

Let's walk through each one.

Vulnerability 1: Unsafe strncpy from Environment Variable (Line ~393)

// BEFORE (vulnerable)
strncpy( telescop, getenv( "TELESCOP" ), FLEN_VALUE );
telescop[FLEN_VALUE - 1]= '\0';

At first glance, this looks safe — strncpy with a length limit, followed by manual null termination. But there's a subtle problem: strncpy does not guarantee null termination when the source string is exactly as long as or longer than the specified limit. The manual null-termination on the next line patches that specific case, but strncpy has another dangerous property: if the source is shorter than n, it pads the remainder with null bytes — wasting cycles — and if it's longer, it silently truncates without any indication of overflow.

More critically, strncpy was never designed for security-sensitive string handling. Its semantics are confusing enough that even experienced C developers misuse it regularly.

The real risk here: If getenv("TELESCOP") returns an attacker-controlled string (e.g., via a crafted execution environment or a wrapper script), a carefully sized value could interact with downstream buffer operations in unexpected ways.

Vulnerability 2: Uninitialized Pointer Array via malloc (Line ~712)

// BEFORE (vulnerable)
wcs_key= malloc( No_of_wcs_keys * sizeof( char * ) );

This allocates an array of char * pointers using malloc, which does not zero-initialize the allocated memory. If any subsequent allocation for wcs_key[i] fails and the error path doesn't clean up perfectly, the uninitialized pointers in the array could be dereferenced — leading to use-after-free or wild pointer dereference conditions.

In a complex error-handling path (which is common in FITS parsing code), an uninitialized pointer array is a landmine waiting to be triggered.

Vulnerability 3: sizeof(char) Multiplication Masking Integer Overflow (Lines ~718–735)

// BEFORE (vulnerable)
wcs_key[0]= (char *)malloc( FLEN_CARD * sizeof( char ) );
memset( wcs_key[0], 0, FLEN_CARD * sizeof( char ) );

// ...in the loop:
wcs_key[i]= (char *)malloc( FLEN_CARD * sizeof( char ) );
memset( wcs_key[i], 0, FLEN_CARD * sizeof( char ) );

sizeof(char) is always 1 in C — so multiplying by it is mathematically a no-op. However, this pattern is dangerous for a subtle reason: it establishes a coding pattern where size calculations involve multiplication, which can mask integer overflow vulnerabilities when the pattern is copy-pasted or extended to use larger types.

If a future developer changes FLEN_CARD to a runtime value or changes the element type, the multiplication pattern is already in place — and if that multiplication overflows a size_t, malloc will allocate a much smaller buffer than expected, leading to a heap overflow.

Additionally, the redundancy adds cognitive noise that can obscure real size calculation bugs during code review.

Vulnerability 4: Unsafe strncpy for Command-Line Arguments (Line ~804)

// BEFORE (vulnerable)
strncpy( fitsfile_name, argv[1], FILENAME_LENGTH );

Same class of problem as Vulnerability 1, but applied to command-line input. An attacker who can control argv[1] (e.g., by invoking the binary with a crafted filename) can supply a string designed to interact with subsequent buffer operations. Combined with the other vulnerabilities, this creates a clear path for exploitation via a malicious FITS filename.

The Attack Scenario

Here's how a sophisticated attacker could chain these vulnerabilities:

1. Craft a malicious FITS file with oversized WCS header fields, carefully sized to interact with the FLEN_CARD-sized buffers allocated for wcs_key.
2. Trigger the uninitialized pointer array by causing a partial allocation failure, leaving some wcs_key[i] pointers uninitialized.
3. Use the unsafe string operations to overwrite adjacent memory, corrupting heap metadata or stack return addresses.
4. Overwrite the instruction pointer with a shellcode address, achieving arbitrary code execution in the context of the process parsing the FITS file.

This is a classic file format exploitation scenario — the same class of attack that has been used against PDF parsers, image decoders, and media processing libraries for decades.

The Fix

The patch addresses each vulnerability with a targeted, idiomatic C security improvement. Let's look at each change:

Fix 1: Replace strncpy with snprintf

// BEFORE
strncpy( telescop, getenv( "TELESCOP" ), FLEN_VALUE );
telescop[FLEN_VALUE - 1]= '\0';

// AFTER
snprintf( telescop, FLEN_VALUE, "%s", getenv( "TELESCOP" ) );
telescop[FLEN_VALUE - 1]= '\0';

snprintf is the correct tool for safe string copying in C. It:
- Always null-terminates the destination buffer (as long as size > 0)
- Never writes more than size bytes (including the null terminator)
- Returns the number of bytes that would have been written, allowing truncation detection

Using "%s" as the format string (rather than passing the environment variable directly as the format) also prevents format string injection — a separate vulnerability class that would arise if the environment variable contained % characters.

The retained telescop[FLEN_VALUE - 1] = '\0' is now redundant but harmless — a good defensive belt-and-suspenders approach.

Fix 2: Replace malloc with calloc for the Pointer Array

// BEFORE
wcs_key= malloc( No_of_wcs_keys * sizeof( char * ) );

// AFTER
wcs_key= calloc( (size_t)No_of_wcs_keys, sizeof( char * ) );

calloc provides two critical security improvements over malloc here:

1. Zero-initialization: All pointers in the array start as NULL. This means any error path that fails to initialize a particular wcs_key[i] will leave a NULL pointer rather than a garbage pointer — making null pointer dereferences far easier to detect and debug than wild pointer dereferences.

2. Built-in overflow protection: calloc(nmemb, size) takes two separate arguments and performs an internal overflow-safe multiplication. If No_of_wcs_keys * sizeof(char *) would overflow a size_t, calloc returns NULL rather than allocating a dangerously undersized buffer.

The explicit (size_t) cast on No_of_wcs_keys is also a good practice — it makes the type intent explicit and prevents signed/unsigned mismatch warnings that could hide real issues.

Fix 3: Remove Redundant sizeof(char) Multiplications

// BEFORE
wcs_key[0]= (char *)malloc( FLEN_CARD * sizeof( char ) );
memset( wcs_key[0], 0, FLEN_CARD * sizeof( char ) );

// AFTER
wcs_key[0]= (char *)malloc( FLEN_CARD );
memset( wcs_key[0], 0, FLEN_CARD );

Removing * sizeof(char) (which always equals * 1) cleans up the code and eliminates the dangerous pattern of size-calculation multiplication in these specific calls. The semantics are identical, but the code is now clearer and less likely to be misused as a template for unsafe size calculations.

Fix 4: Replace strncpy with snprintf for Filename Handling

// BEFORE
strncpy( fitsfile_name, argv[1], FILENAME_LENGTH );

// AFTER
snprintf( /* fitsfile_name, FILENAME_LENGTH, "%s", argv[1] */ );

Same rationale as Fix 1 — snprintf provides guaranteed null termination and safe truncation semantics for the command-line filename argument.

Prevention & Best Practices

1. Never Use strncpy for Security-Sensitive String Copying

strncpy was designed for a specific historical use case (fixed-width record fields) and is almost always the wrong choice for modern C code. Use snprintf instead:

// Dangerous
strncpy(dest, src, sizeof(dest));

// Safe
snprintf(dest, sizeof(dest), "%s", src);

2. Prefer calloc Over malloc for Arrays

When allocating arrays — especially arrays of pointers — use calloc:

// Risky: uninitialized memory, no overflow check
char **arr = malloc(n * sizeof(char *));

// Safe: zero-initialized, overflow-safe multiplication
char **arr = calloc((size_t)n, sizeof(char *));

3. Validate All External Input Before Buffer Operations

Any data that crosses a trust boundary — file contents, environment variables, command-line arguments, network data — must be validated before being copied into fixed-size buffers. Define and enforce maximum lengths explicitly.

4. Use Static Analysis Tools

Tools that can catch these vulnerabilities automatically include:

• Clang Static Analyzer — detects buffer overflows, use-after-free, and memory leaks
• Coverity — enterprise-grade static analysis for C/C++
• AddressSanitizer (ASan) — runtime detection of buffer overflows and use-after-free
• Valgrind — memory error detection and profiling
• cppcheck — lightweight static analysis for C/C++

5. Enable Compiler Hardening Flags

When building C code, enable security-relevant compiler flags:

# GCC/Clang hardening flags
-Wall -Wextra -Werror
-D_FORTIFY_SOURCE=2
-fstack-protector-strong
-fPIE -pie
-Wformat -Wformat-security

These flags enable stack canaries, format string warnings, and ASLR support that make exploitation significantly harder even if vulnerabilities exist.

6. Follow the CERT C Secure Coding Standard

The CERT C Secure Coding Standard provides specific rules for avoiding these vulnerability classes:

• STR07-C: Use the bounds-checking interfaces for string manipulation
• MEM04-C: Beware of zero-length allocations
• INT30-C: Ensure that unsigned integer operations do not wrap
• MEM35-C: Allocate sufficient memory for an object

7. Fuzz Test File Parsers

File format parsers are a prime target for fuzzing. Tools like AFL++ and libFuzzer can automatically generate malformed FITS files to discover buffer handling bugs before attackers do.

Conclusion

The vulnerabilities fixed in try_to_guess_image_fov.c are a textbook example of how small, individually subtle C programming mistakes compound into critical security risks. No single line of code was obviously catastrophic — but together, they created a clear path from a malicious FITS file to arbitrary code execution.

The key takeaways for C developers are:

• strncpy is not safe — use snprintf for bounded string copying
• malloc leaves memory uninitialized — use calloc for arrays, especially pointer arrays
• sizeof(char) multiplications are noise — remove them to keep size calculations clear and auditable
• External input is always hostile — validate lengths before any buffer operation
• Layer your defenses — combine safe APIs, compiler flags, static analysis, and fuzzing

Security in C isn't about a single magic function or pattern — it's about consistent, disciplined application of safe practices across every line that touches external data. Automated security scanning, as demonstrated by this fix, plays a critical role in catching the issues that inevitably slip through manual review.

This vulnerability was identified and fixed by OrbisAI Security. Automated security scanning can find issues like this before they reach production — consider integrating continuous security analysis into your CI/CD pipeline.