Fix out-of-bounds write in Vector initializer_list constructor

[CodeReclaimers]

Problem

The initializer_list constructor in Vector.h (line 82, enabled for N >= 5) has a loop bound bug that causes two problems:

1. Buffer overflow when more than N initializers are provided:

The second loop at line 82 uses i < numValues as its bound. When numValues > N, the first loop correctly copies only min(numValues, N) elements, but the second loop then continues writing zeros to mTuple[N], mTuple[N+1], ..., mTuple[numValues-1] — all past the end of the fixed-size std::array<Real, N>.

2. Missing zero-fill when fewer than N initializers are provided:

The comment on line 66 documents the intent: "At most N elements are copied from the initializer list, setting any remaining elements to zero." However, when numValues < N, the second loop condition i < numValues is immediately false (since i == numValues after the first loop), so elements [numValues..N-1] are left uninitialized.

Fix

One-character change: numValues → N on line 82.

-            for (; i < numValues; ++i)
+            for (; i < N; ++i)

This fixes both problems: the loop now stops at N (preventing overflow) and runs from numValues to N (zeroing remaining elements).

Test program

The test below demonstrates both bugs against the unfixed code and passes cleanly after the fix.

To build: place both files in a directory, run cmake . -B build && cmake --build build, then ./build/test_issue_1_1.

Output BEFORE fix:

Test 1: Out-of-bounds write when initializer_list size > N
  CORRUPTION: sentinel[0] changed from -6.27744e+66 to 0
  CORRUPTION: sentinel[1] changed from -6.27744e+66 to 0
  CORRUPTION: sentinel[2] changed from -6.27744e+66 to 0
  FAIL: Out-of-bounds write detected!

Test 2: Missing zero-fill when initializer_list size < N
  NOT ZEROED: vec[3] = -nan (expected 0.0)
  NOT ZEROED: vec[4] = -nan (expected 0.0)
  ...
  FAIL: Elements beyond the initializer list were NOT zeroed.

2 bug(s) detected out of 3 tests.

Output AFTER fix:

Test 1 (OOB write):       passed
Test 2 (missing zeroing):  passed
Test 3 (exact match):      passed

0 bug(s) detected out of 3 tests.

CMakeLists.txt

cmake_minimum_required(VERSION 3.14)
project(test_issue_1_1 LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

# GTE headers are in ../../GTE relative to this directory.
set(GTE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../GTE")

add_executable(test_issue_1_1 test_issue_1_1.cpp)
target_include_directories(test_issue_1_1 PRIVATE "${GTE_ROOT}")

test_issue_1_1.cpp

// Test for Issue 1.1: Vector.h initializer_list constructor out-of-bounds write
//
// Bug: Vector<N,Real>'s initializer_list constructor (enabled for N >= 5)
// has a second loop that iterates up to `numValues` instead of `N`.
// When more initializers are provided than the vector dimension N, the loop
// writes past the end of the fixed-size mTuple array (buffer overflow).
// Additionally, when fewer initializers are provided than N, the remaining
// elements are left uninitialized instead of being zeroed.
//
// File: GTE/Mathematics/Vector.h, lines 82-85
//   for (; i < numValues; ++i)      // BUG: should be i < N
//       mTuple[i] = static_cast<Real>(0);
//
// Expected fix: Change `i < numValues` to `i < N`

#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <iostream>
#include <array>

// Include the Vector header from GTE
#include <Mathematics/Vector.h>

// ---------------------------------------------------------------------------
// Test 1: Out-of-bounds write when numValues > N
//
// We place a Vector<5> inside a struct alongside a sentinel array that sits
// in memory right after it. If the constructor writes past element [4], the
// sentinel will be overwritten.
// ---------------------------------------------------------------------------
struct AlignedLayout
{
    // The Vector<5, double> occupies 5 doubles = 40 bytes.
    gte::Vector<5, double> vec;

    // Sentinel immediately after the vector's storage.
    // Fill with a known pattern; if the constructor overwrites past element [4],
    // some of these bytes will change.
    double sentinel[4];

    AlignedLayout()
    {
        // Pre-fill everything with a known bit pattern (0xCD bytes).
        std::memset(this, 0xCD, sizeof(*this));
    }
};

static bool test_oob_write()
{
    std::cout << "Test 1: Out-of-bounds write when initializer_list size > N\n";
    std::cout << "  Creating AlignedLayout with sentinel after Vector<5>...\n";

    AlignedLayout layout;

    // Verify sentinel is intact before constructing the vector.
    double sentinel_backup[4];
    std::memcpy(sentinel_backup, layout.sentinel, sizeof(sentinel_backup));

    // Now construct a Vector<5, double> with 8 initializers (3 too many).
    // The first loop copies min(8, 5) = 5 elements into mTuple[0..4].
    // The buggy second loop then writes mTuple[5], mTuple[6], mTuple[7]
    // with zeros -- 3 writes past the end of the array.
    //
    // We use placement new to construct into the existing layout.
    new (&layout.vec) gte::Vector<5, double>(
        { 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0 }
    );

    // Check if sentinel was corrupted.
    bool sentinel_corrupted = false;
    for (int i = 0; i < 4; ++i)
    {
        if (std::memcmp(&layout.sentinel[i], &sentinel_backup[i], sizeof(double)) != 0)
        {
            sentinel_corrupted = true;
            std::cout << "  CORRUPTION: sentinel[" << i << "] changed from "
                      << sentinel_backup[i] << " to " << layout.sentinel[i] << "\n";
        }
    }

    if (sentinel_corrupted)
    {
        std::cout << "  FAIL: Out-of-bounds write detected! The initializer_list\n"
                  << "        constructor wrote past the end of mTuple.\n";
    }
    else
    {
        std::cout << "  PASS: Sentinel was not corrupted.\n";
    }

    // Also verify the vector contents are correct (first 5 elements).
    bool values_ok = true;
    for (int i = 0; i < 5; ++i)
    {
        if (layout.vec[i] != static_cast<double>(i + 1))
        {
            values_ok = false;
            std::cout << "  NOTE: vec[" << i << "] = " << layout.vec[i]
                      << ", expected " << (i + 1) << "\n";
        }
    }
    if (values_ok)
    {
        std::cout << "  Vector elements [0..4] are correct: {1, 2, 3, 4, 5}\n";
    }

    return sentinel_corrupted;  // true = bug detected
}

// ---------------------------------------------------------------------------
// Test 2: Uninitialized elements when numValues < N
//
// When fewer initializers are provided than N, the second loop (which should
// zero-fill the rest) doesn't execute because it checks i < numValues, and
// i already equals numValues after the first loop. Elements beyond the
// provided initializers are left uninitialized.
// ---------------------------------------------------------------------------
static bool test_missing_zero_fill()
{
    std::cout << "\nTest 2: Missing zero-fill when initializer_list size < N\n";
    std::cout << "  Creating Vector<8, double> with only 3 initializers...\n";

    // Pre-fill memory with a known non-zero pattern so we can detect
    // whether the constructor zeroed the remaining elements.
    alignas(alignof(gte::Vector<8, double>)) unsigned char
        storage[sizeof(gte::Vector<8, double>)];
    std::memset(storage, 0xFF, sizeof(storage));

    // Construct a Vector<8> with only 3 initializers.
    // Expected: elements [0..2] = {10, 20, 30}, elements [3..7] = 0.
    // Actual (bug): elements [3..7] retain the 0xFF pattern.
    auto* vec = new (storage) gte::Vector<8, double>({ 10.0, 20.0, 30.0 });

    bool has_uninitialized = false;

    std::cout << "  Vector contents:\n";
    for (int i = 0; i < 8; ++i)
    {
        std::cout << "    vec[" << i << "] = " << (*vec)[i] << "\n";
    }

    // Check that elements [0..2] have the correct values.
    bool init_ok = ((*vec)[0] == 10.0 && (*vec)[1] == 20.0 && (*vec)[2] == 30.0);
    if (!init_ok)
    {
        std::cout << "  NOTE: Initial elements are wrong.\n";
    }

    // Check that elements [3..7] are zero (as the comment on line 66 promises).
    for (int i = 3; i < 8; ++i)
    {
        if ((*vec)[i] != 0.0)
        {
            has_uninitialized = true;
            std::cout << "  NOT ZEROED: vec[" << i << "] = " << (*vec)[i]
                      << " (expected 0.0)\n";
        }
    }

    if (has_uninitialized)
    {
        std::cout << "  FAIL: Elements beyond the initializer list were NOT zeroed.\n"
                  << "        The comment says 'setting any remaining elements to zero'\n"
                  << "        but the second loop checks i < numValues instead of i < N.\n";
    }
    else
    {
        std::cout << "  PASS: All remaining elements are zero.\n";
    }

    vec->~Vector();
    return has_uninitialized;  // true = bug detected
}

// ---------------------------------------------------------------------------
// Test 3: Exact match (numValues == N) -- should work correctly
// ---------------------------------------------------------------------------
static bool test_exact_match()
{
    std::cout << "\nTest 3: Exact match (initializer_list size == N) -- baseline\n";

    gte::Vector<5, double> vec({ 1.0, 2.0, 3.0, 4.0, 5.0 });

    bool ok = true;
    for (int i = 0; i < 5; ++i)
    {
        if (vec[i] != static_cast<double>(i + 1))
        {
            ok = false;
            std::cout << "  UNEXPECTED: vec[" << i << "] = " << vec[i]
                      << ", expected " << (i + 1) << "\n";
        }
    }

    if (ok)
    {
        std::cout << "  PASS: All 5 elements are correct.\n";
    }
    else
    {
        std::cout << "  FAIL: Unexpected values.\n";
    }

    return !ok;  // true = bug detected
}

// ---------------------------------------------------------------------------
int main()
{
    std::cout << "=== Issue 1.1: Vector<N> initializer_list out-of-bounds write ===\n";
    std::cout << "File: GTE/Mathematics/Vector.h, line 82\n";
    std::cout << "Bug: Second loop uses 'i < numValues' instead of 'i < N'\n\n";

    bool bug1 = test_oob_write();
    bool bug2 = test_missing_zero_fill();
    bool bug3 = test_exact_match();

    std::cout << "\n=== Summary ===\n";
    std::cout << "Test 1 (OOB write):       " << (bug1 ? "BUG DETECTED" : "passed") << "\n";
    std::cout << "Test 2 (missing zeroing):  " << (bug2 ? "BUG DETECTED" : "passed") << "\n";
    std::cout << "Test 3 (exact match):      " << (bug3 ? "BUG DETECTED" : "passed") << "\n";

    int bugs_found = (bug1 ? 1 : 0) + (bug2 ? 1 : 0) + (bug3 ? 1 : 0);
    std::cout << "\n" << bugs_found << " bug(s) detected out of 3 tests.\n";

    return bugs_found > 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}

🤖 Generated with Claude Code