Skip to content

Fix buffer overrun in SingularValueDecomposition::GetV#120

Open
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/svd-getv-size
Open

Fix buffer overrun in SingularValueDecomposition::GetV#120
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/svd-getv-size

Conversation

@CodeReclaimers
Copy link
Contributor

GetV (line 236) loops over mUMatrix.size() (M*M) instead of mVMatrix.size() (N*N). When M > N, this reads past the end of mVMatrix.

Fix: change mUMatrix.size() to mVMatrix.size() on line 236.

Test (CMakeLists.txt)
cmake_minimum_required(VERSION 3.14)
project(test_issue_7_1 LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(GTE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../GTE")
add_executable(test_issue_7_1 test_issue_7_1.cpp)
target_include_directories(test_issue_7_1 PRIVATE "${GTE_ROOT}")
Test (test_issue_7_1.cpp)
#include <cmath>
#include <cstdlib>
#include <iostream>
#include <vector>
#include <Mathematics/SingularValueDecomposition.h>

static bool test_getv_size()
{
    using Real = double;
    size_t const M = 5, N = 3;
    gte::SingularValueDecomposition<Real> svd(M, N, 1000);

    std::vector<Real> A(M * N);
    for (size_t r = 0; r < M; ++r)
        for (size_t c = 0; c < N; ++c)
            A[c + N * r] = static_cast<Real>((r+1)*(c+1));
    svd.Solve(A.data());

    // Surround buffer with sentinels to detect overrun.
    Real const sentinel = -999.0;
    std::vector<Real> vBuf(N*N + 4, sentinel);
    Real* vMatrix = vBuf.data() + 2;
    svd.GetV(vMatrix);

    return vBuf[0] == sentinel && vBuf[1] == sentinel
        && vBuf[N*N+2] == sentinel && vBuf[N*N+3] == sentinel;
}

int main()
{
    return test_getv_size() ? EXIT_SUCCESS : EXIT_FAILURE;
}

GetV looped over mUMatrix.size() (M*M) instead of mVMatrix.size() (N*N),
reading past the end of mVMatrix when M > N.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant