Critical Buffer Overflow in gravier-str.h: How Broken Bit Shifts and Missing Bounds Checks Created a Memory Corruption Vulnerability

Introduction

Buffer overflows are among the oldest and most dangerous vulnerability classes in software security. Despite decades of awareness, they continue to appear in modern codebases — often hiding in utility code that developers trust implicitly. This post examines a critical-severity buffer overflow discovered in gravier/gravier-str.h, a string management utility that underpins user-facing input handling and scripting engine integration.

What makes this vulnerability particularly instructive is that it wasn't caused by a single mistake. It was the result of three compounding bugs — a typo that silently broke a core math function, an allocation that forgot to account for the null terminator, and reallocation logic that checked the wrong condition. Together, they created a reliable path to heap corruption.

If you write C or C++, or maintain any code that performs manual string management, this is a vulnerability worth understanding deeply.

The Vulnerability Explained

Background: What is gravier_str?

gravier_str is a dynamic string type — a common pattern in C codebases that lack a built-in string class. It wraps a char * pointer alongside a length field, and provides functions like gravier_str_init, gravier_str_append, and gravier_str_replace to manage the string's lifetime and growth.

The design relies on a helper function, next_power_of_2(), to calculate buffer sizes. The idea is simple: always allocate a power-of-two number of bytes so that reallocations are infrequent and predictable.

Unfortunately, this helper function was silently broken.

Bug #1: The Broken Bit-Shift Typo

Here is the original next_power_of_2() implementation:

// VULNERABLE CODE
static inline int32_t next_power_of_2(int32_t i)
{
    --i;
    i |= 1 >> 1;   // ← Bug: shifts the literal 1, not i
    i |= 1 >> 2;   // ← Bug
    i |= 1 >> 4;   // ← Bug
    i |= 1 >> 8;   // ← Bug
    i |= 1 >> 16;  // ← Bug
    ++i;
    return i;
}

The classic bit-spreading algorithm for finding the next power of two works by OR-ing a value with right-shifted copies of itself. Each shift propagates the highest set bit downward, filling all lower bits with ones — so that ++i produces the next power of two.

The bug here is a single-character typo: 1 >> N instead of i >> N. Because integer literal 1 is being shifted rather than i, the result is always 0 (right-shifting 1 by 1 or more positions yields 0 in integer arithmetic). This means the function effectively becomes:

--i;
i |= 0;  // no-op, five times
++i;
return i;  // returns the original value unchanged

The function no longer computes the next power of two — it just returns its input. For most inputs, this produces an undersized buffer. For example, if gstr->len is 10, the function should return 16, but instead returns 10 — and the allocation is too small.

Bug #2: Off-by-One in the Initial Allocation

Even if next_power_of_2() had worked correctly, there was a second problem in gravier_str_init:

// VULNERABLE CODE
gstr->len = strlen(base);
gstr->str = malloc(next_power_of_2(gstr->len));  // ← no +1 for null terminator
strcpy(gstr->str, base);

strlen() returns the number of characters in the string, not including the null terminator (\0). When you allocate malloc(strlen(base)) bytes and then strcpy into it, you write one byte past the end of the allocated buffer — a classic off-by-one heap overflow.

For a 7-character string like "gravier", strlen returns 7. You allocate 7 bytes (or, with the broken power-of-two function, possibly even fewer). strcpy writes 8 bytes (7 characters + null terminator), overwriting one byte of adjacent heap memory.

Bug #3: Incorrect Reallocation Guard

The append functions also contained flawed growth logic:

// VULNERABLE CODE
int gravier_str_append(struct gravier_str *gstr, char *str) {
    int other_len = strlen(str);
    int next_big = next_power_of_2(gstr->len);  // ← doesn't account for other_len

    if (gstr->len + other_len >= next_big) {     // ← checks wrong threshold
        char *tmp = malloc(next_big);
        // ...
    }
    strcpy(gstr->str + gstr->len, str);          // ← writes without bounds check
}

There are two problems here:

1. next_big is calculated from gstr->len alone, ignoring the length of the string being appended. So even when a reallocation is triggered, the new buffer may still be too small to hold the combined string.

2. The condition gstr->len + other_len >= next_big may not trigger when it should, because next_big (being computed from only gstr->len) can be larger than the actual current buffer size, masking the overflow condition.

The strcpy at the end then writes into a buffer that hasn't been grown sufficiently — a reliable heap write-out-of-bounds.

Real-World Impact

This vulnerability is directly reachable from:

• User-facing menu text input — any string typed by the user can flow into these functions
• The s7 scripting engine — scripts can construct and manipulate strings programmatically

An attacker who can supply crafted input (e.g., a long menu label or a script that builds long strings) can:

• Corrupt adjacent heap allocations, leading to application crashes
• Overwrite heap metadata, potentially leading to arbitrary code execution
• Bypass ASLR in some heap exploitation scenarios by leveraging predictable allocation patterns

This maps to CWE-122: Heap-based Buffer Overflow and is rated Critical (CVSS 9.x range) when user-controlled input is involved.

The Fix

The patch addresses all three bugs with targeted, minimal changes.

Fix #1: Correct the Bit-Shift Operations

// FIXED CODE
static inline int32_t next_power_of_2(int32_t i)
{
    --i;
    i |= i >> 1;   // ← now correctly shifts i
    i |= i >> 2;
    i |= i >> 4;
    i |= i >> 8;
    i |= i >> 16;
    ++i;
    return i;
}

Changing 1 >> N to i >> N restores the intended bit-spreading behavior. Now the function correctly returns the smallest power of two that is greater than or equal to its input. For example:
- Input 10 → returns 16
- Input 16 → returns 16
- Input 17 → returns 32

This ensures all allocations are at least as large as needed, with room to spare.

Fix #2: Account for the Null Terminator

// FIXED CODE
gstr->len = strlen(base);
gstr->str = malloc(next_power_of_2(gstr->len + 1));  // ← +1 for '\0'
strcpy(gstr->str, base);

Adding + 1 before passing to next_power_of_2 ensures the allocated buffer always has room for the null terminator. This is the single most common off-by-one mistake in C string handling, and it's now correctly addressed.

Fix #3: Correct the Reallocation Logic

// FIXED CODE
int gravier_str_append(struct gravier_str *gstr, char *str) {
    int other_len = strlen(str);
    int next_big = next_power_of_2(gstr->len + other_len + 1);  // ← full combined size

    if (next_big > next_power_of_2(gstr->len + 1)) {  // ← compares actual buffer sizes
        char *tmp = malloc(next_big);
        // ...
    }
    strcpy(gstr->str + gstr->len, str);
}

The same pattern is applied consistently to gravier_str_append_int and gravier_str_append_float:

// FIXED: gravier_str_append_int
int other_len = 64;
int next_big = next_power_of_2(gstr->len + other_len + 1);

if (next_big > next_power_of_2(gstr->len + 1)) {
    // reallocate
}

// FIXED: gravier_str_append_float
int other_len = 128;
int next_big = next_power_of_2(gstr->len + other_len + 1);

if (next_big > next_power_of_2(gstr->len + 1)) {
    // reallocate
}

Now next_big represents the buffer size needed for the combined string (current content + new content + null terminator). The condition compares this against the size of the current buffer (computed as next_power_of_2(gstr->len + 1)). If the required size exceeds the current allocation, a reallocation occurs — and the new buffer is guaranteed to be large enough.

Before vs. After Summary

Prevention & Best Practices

This vulnerability is a textbook example of why C string handling deserves extra scrutiny. Here's how to prevent similar issues in your own code:

1. Prefer strlcpy and strncat Over strcpy and strcat

// Dangerous
strcpy(dest, src);

// Safer
strlcpy(dest, src, dest_size);

strlcpy (available on BSD and via libbsd on Linux) always null-terminates and never writes beyond dest_size bytes. It's not a perfect solution, but it prevents the worst outcomes.

2. Always Allocate strlen(s) + 1

This is a rule with no exceptions in C:

char *copy = malloc(strlen(original) + 1);  // +1 is mandatory

Consider writing a wrapper that enforces this:

char *safe_strdup(const char *s) {
    size_t len = strlen(s) + 1;
    char *copy = malloc(len);
    if (copy) memcpy(copy, s, len);
    return copy;
}

3. Test Your Math Utilities Independently

The next_power_of_2 bug could have been caught by a single unit test:

assert(next_power_of_2(1)  == 1);
assert(next_power_of_2(5)  == 8);
assert(next_power_of_2(16) == 16);
assert(next_power_of_2(17) == 32);

Utility functions that are used in security-sensitive contexts (memory allocation, bounds checking) deserve their own test suite.

4. Use AddressSanitizer (ASan) During Development

Compile with -fsanitize=address to catch heap overflows at runtime during testing:

gcc -fsanitize=address -g -o myapp myapp.c

ASan would have immediately flagged the off-by-one write in gravier_str_init the first time it was exercised.

5. Consider Safer Abstractions

If your project allows it, consider replacing manual string buffers with well-tested libraries:

• C: SDS (Simple Dynamic Strings), used by Redis
• C++: std::string with .append() and .reserve()
• Rust: String and &str with compile-time memory safety guarantees

6. Relevant Security Standards

• CWE-122: Heap-based Buffer Overflow
• CWE-131: Incorrect Calculation of Buffer Size
• CWE-193: Off-by-one Error
• OWASP: Buffer Overflow
• SEI CERT C: STR31-C: Guarantee that storage for strings has sufficient space

Conclusion

This vulnerability is a reminder that critical security bugs often hide in the most mundane utility code. A one-character typo (1 >> instead of i >>) silently disabled a buffer-sizing function, and two additional allocation mistakes compounded the damage into a reliable, user-reachable heap overflow.

The key takeaways for developers:

• Test your math. Functions that compute sizes or offsets should have unit tests with boundary values.
• Always include the null terminator in C string allocations. No exceptions.
• Reallocation logic must account for the full combined size, not just the current size.
• Use sanitizers. ASan and Valgrind exist precisely to catch these bugs before they reach production.
• Treat user-reachable string paths as attack surfaces. Any code that processes external input deserves extra scrutiny.

The fix here was small — a handful of lines — but the impact of leaving it unfixed could have been severe. Automated security scanning caught what code review missed, and the patch was verified with both a build check and a re-scan. That's the security development lifecycle working as intended.

This vulnerability was identified and fixed by OrbisAI Security. Automated scanning, AI-assisted code review, and developer education work best together.