From c9217b6e32939d0b7144f6f6cc85bfa94194ec8b Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Mon, 21 Sep 2026 11:53:56 -0400 Subject: [PATCH 1/3] Stop reading values once the declared count is reached createArrayFromFile and createVectorFromFile size the allocation from the Matrix Market header and then read to end of file, so a body longer than the header declares is written past the end. updateVectorFromFile compares the declared length against the vector size and then does the same, so a header that matches exactly plus extra values passes the check and still overflows. Signed-off-by: Arpit Jain --- resolve/matrix/io.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/resolve/matrix/io.cpp b/resolve/matrix/io.cpp index b48e34b63..46521292f 100644 --- a/resolve/matrix/io.cpp +++ b/resolve/matrix/io.cpp @@ -267,7 +267,9 @@ namespace ReSolve real_type* vec = new real_type[n]; real_type a; - while (file >> a) + // Stop at the number of values the header declares; a longer body would + // otherwise be written past the end of the allocation. + while (i < n && file >> a) { vec[i] = a; i++; @@ -299,7 +301,8 @@ namespace ReSolve vector::Vector* vec = new vector::Vector(n); vec->allocate(memory::HOST); real_type a; - while (file >> a) + // As above, the header decides how many values fit. + while (i < n && file >> a) { vec->getData(memory::HOST)[i] = a; i++; @@ -446,7 +449,9 @@ namespace ReSolve real_type* rhs = vec_rhs->getData(memory::HOST); real_type a = 0.0; index_type i = 0; - while (file >> a) + // The size check above compares the declared length, not how many values + // the body actually carries, so the loop is bounded here too. + while (i < n && file >> a) { rhs[i] = a; // std::cout << i << ": " << a << "\n"; From ef734b6b31048b81e3e1fa79929003007cedae5f Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Mon, 21 Sep 2026 11:54:41 -0400 Subject: [PATCH 2/3] Bound updateArrayFromFile the same way The fourth reader in this file has the same unbounded loop and carries a todo from the maintainers saying the risk of writing past the end is high. Signed-off-by: Arpit Jain --- resolve/matrix/io.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/resolve/matrix/io.cpp b/resolve/matrix/io.cpp index 46521292f..9172990ec 100644 --- a/resolve/matrix/io.cpp +++ b/resolve/matrix/io.cpp @@ -411,7 +411,9 @@ namespace ReSolve } real_type a; index_type i = 0; - while (file >> a) + // The caller's buffer holds n values when this function allocated it, and + // the header is all there is to go on when it did not. + while (i < n && file >> a) { rhs[i] = a; i++; From 1c01fee6350e1f6168810989db587b7b8daeb3bd Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 29 Sep 2026 10:37:12 -0400 Subject: [PATCH 3/3] Reject a vector file whose body does not match its header Signed-off-by: Arpit Jain --- resolve/matrix/io.cpp | 121 ++++++++++++++++-------- tests/unit/matrix/MatrixIoTests.hpp | 126 +++++++++++++++++++++++++ tests/unit/matrix/runMatrixIoTests.cpp | 1 + 3 files changed, 210 insertions(+), 38 deletions(-) diff --git a/resolve/matrix/io.cpp b/resolve/matrix/io.cpp index 9172990ec..600b5a5e3 100644 --- a/resolve/matrix/io.cpp +++ b/resolve/matrix/io.cpp @@ -1,11 +1,13 @@ #include "io.hpp" +#include #include #include #include #include #include #include +#include #include #include @@ -148,10 +150,11 @@ namespace ReSolve matrix::Sparse* A, std::list& tmp); // static void print_list(std::list& l); - static int loadToList(std::istream& file, bool is_expand_symmetric, std::list& tmp); - static int removeDuplicates(std::list& tmp); - static int copyListToCoo(const std::list& tmp, matrix::Coo* A); - static int copyListToCsr(const std::list& tmp, matrix::Csr* A); + static int loadToList(std::istream& file, bool is_expand_symmetric, std::list& tmp); + static int removeDuplicates(std::list& tmp); + static int copyListToCoo(const std::list& tmp, matrix::Coo* A); + static int copyListToCsr(const std::list& tmp, matrix::Csr* A); + static bool readArrayBody(std::istream& file, index_type n, std::vector& values); /** * @brief Create a COO matrix and populate it with data from Matrix Market @@ -254,7 +257,6 @@ namespace ReSolve std::stringstream ss; std::string line; - index_type i = 0; index_type n, m; std::getline(file, line); @@ -265,15 +267,14 @@ namespace ReSolve ss << line; ss >> n >> m; - real_type* vec = new real_type[n]; - real_type a; - // Stop at the number of values the header declares; a longer body would - // otherwise be written past the end of the allocation. - while (i < n && file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - vec[i] = a; - i++; + return nullptr; } + + real_type* vec = new real_type[n]; + std::copy(values.begin(), values.end(), vec); return vec; } @@ -287,7 +288,6 @@ namespace ReSolve std::stringstream ss; std::string line; - index_type i = 0; index_type n, m; std::getline(file, line); @@ -298,15 +298,15 @@ namespace ReSolve ss << line; ss >> n >> m; - vector::Vector* vec = new vector::Vector(n); - vec->allocate(memory::HOST); - real_type a; - // As above, the header decides how many values fit. - while (i < n && file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - vec->getData(memory::HOST)[i] = a; - i++; + return nullptr; } + + vector::Vector* vec = new vector::Vector(n); + vec->allocate(memory::HOST); + std::copy(values.begin(), values.end(), vec->getData(memory::HOST)); vec->setDataUpdated(memory::HOST); return vec; } @@ -405,19 +405,18 @@ namespace ReSolve ss << line; ss >> n >> m; - if (rhs == nullptr) + std::vector values; + if (!readArrayBody(file, n, values)) { - rhs = new real_type[n]; + Logger::error() << "Array not updated!\n"; + return; } - real_type a; - index_type i = 0; - // The caller's buffer holds n values when this function allocated it, and - // the header is all there is to go on when it did not. - while (i < n && file >> a) + + if (rhs == nullptr) { - rhs[i] = a; - i++; + rhs = new real_type[n]; } + std::copy(values.begin(), values.end(), rhs); } void updateVectorFromFile(std::istream& file, vector::Vector* vec_rhs) @@ -448,17 +447,14 @@ namespace ReSolve return; } - real_type* rhs = vec_rhs->getData(memory::HOST); - real_type a = 0.0; - index_type i = 0; - // The size check above compares the declared length, not how many values - // the body actually carries, so the loop is bounded here too. - while (i < n && file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - rhs[i] = a; - // std::cout << i << ": " << a << "\n"; - i++; + Logger::error() << "Vector not updated!\n"; + return; } + + std::copy(values.begin(), values.end(), vec_rhs->getData(memory::HOST)); vec_rhs->setDataUpdated(memory::HOST); } @@ -528,6 +524,55 @@ namespace ReSolve // // Static helper functions + + /** + * @brief Reads the body of a Matrix Market vector into `values`. + * + * The header says how many entries the body has. Reading fewer or more + * than that means the file is malformed, so nothing is returned to the + * caller in that case. Trailing blank lines are not entries; the + * extraction operator skips them. + * + * @param file - input stream positioned at the first entry. + * @param n - number of entries declared in the header. + * @param values - output, filled only when the body matches the header. + * @return true if the body holds exactly `n` entries. + */ + static bool readArrayBody(std::istream& file, index_type n, std::vector& values) + { + values.clear(); + if (n < 0) + { + Logger::error() << "Matrix market header declares a negative length " << n << ".\n"; + return false; + } + values.reserve(static_cast(n)); + + real_type a; + while (values.size() < static_cast(n) && file >> a) + { + values.push_back(a); + } + + if (values.size() < static_cast(n)) + { + Logger::error() << "Matrix market header declares " << n << " entries but only " + << values.size() << " were read.\n"; + values.clear(); + return false; + } + + if (file >> a) + { + Logger::error() << "Matrix market header declares " << n + << " entries but the body has more.\n"; + values.clear(); + return false; + } + + return true; + } + // /** diff --git a/tests/unit/matrix/MatrixIoTests.hpp b/tests/unit/matrix/MatrixIoTests.hpp index e5505e31c..8c42a1aa6 100644 --- a/tests/unit/matrix/MatrixIoTests.hpp +++ b/tests/unit/matrix/MatrixIoTests.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -454,6 +455,96 @@ namespace ReSolve return status.report(__func__); } + /** + * @brief Reject a vector file whose body does not match its header. + * + * The header of a Matrix Market vector says how many entries follow. + * A file with fewer or more than that is malformed, so the readers + * return nothing and leave the caller's data alone. + */ + TestOutcome rhsVectorLengthMismatch() + { + TestStatus status; + status = true; + + // These cases all log an error on purpose, so keep the output quiet. + ReSolve::io::Logger::setVerbosity(ReSolve::io::Logger::NONE); + + const std::string bad_files[2] = {short_vector_file_, long_vector_file_}; + const char* bad_names[2] = {"short", "long"}; + + for (int k = 0; k < 2; ++k) + { + std::istringstream array_file(bad_files[k]); + real_type* rhs_array = ReSolve::io::createArrayFromFile(array_file); + if (rhs_array != nullptr) + { + std::cout << "createArrayFromFile accepted a " << bad_names[k] << " body.\n"; + status = false; + delete[] rhs_array; + } + + std::istringstream vector_file(bad_files[k]); + vector::Vector* rhs_vector = ReSolve::io::createVectorFromFile(vector_file); + if (rhs_vector != nullptr) + { + std::cout << "createVectorFromFile accepted a " << bad_names[k] << " body.\n"; + status = false; + delete rhs_vector; + } + + // An update leaves the target untouched, so the sentinel survives. + index_type N = static_cast(general_vector_vals_.size()); + vector::Vector vec_rhs(N); + vec_rhs.allocate(memory::HOST); + real_type* data = vec_rhs.getData(memory::HOST); + for (index_type i = 0; i < N; ++i) + { + data[i] = -1.0; + } + + std::istringstream update_file(bad_files[k]); + ReSolve::io::updateVectorFromFile(update_file, &vec_rhs); + for (index_type i = 0; i < N; ++i) + { + if (!isEqual(vec_rhs.getData(memory::HOST)[i], -1.0)) + { + std::cout << "updateVectorFromFile wrote data from a " << bad_names[k] + << " body at element " << i << ".\n"; + status = false; + break; + } + } + } + + ReSolve::io::Logger::setVerbosity(ReSolve::io::Logger::WARNINGS); + + // Blank lines after the last entry are not entries. + std::istringstream padded_file(padded_vector_file_); + vector::Vector* padded = ReSolve::io::createVectorFromFile(padded_file); + if (padded == nullptr) + { + std::cout << "createVectorFromFile rejected a file with trailing blank lines.\n"; + status = false; + } + else + { + const real_type* padded_data = padded->getData(memory::HOST); + for (size_t i = 0; i < general_vector_vals_.size(); ++i) + { + if (!isEqual(padded_data[i], general_vector_vals_[i])) + { + std::cout << "Incorrect vector value at storage element " << i << ".\n"; + status = false; + break; + } + } + delete padded; + } + + return status.report(__func__); + } + private: bool verifyAnswer(/* const */ ReSolve::matrix::Coo& answer, const std::vector& row_data, @@ -737,6 +828,41 @@ namespace ReSolve 3.000e-02 4.000e+00 5.505e+02 +)"; + + /// Header says five entries, body has three. + const std::string short_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 +)"; + + /// Header says five entries, body has seven. + const std::string long_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 + 4.000e+00 + 5.505e+02 + 6.000e+00 + 7.000e+00 +)"; + + /// Five entries, then blank lines that are not entries. + const std::string padded_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 + 4.000e+00 + 5.505e+02 + + )"; const std::vector general_vector_vals_ = {1.000e+00, diff --git a/tests/unit/matrix/runMatrixIoTests.cpp b/tests/unit/matrix/runMatrixIoTests.cpp index 7915b0909..f3b2f57e4 100644 --- a/tests/unit/matrix/runMatrixIoTests.cpp +++ b/tests/unit/matrix/runMatrixIoTests.cpp @@ -22,6 +22,7 @@ int main(int, char**) result += test.rhsVectorReadFromFile(); result += test.rhsArrayReadAndUpdate(); result += test.rhsVectorReadAndUpdate(); + result += test.rhsVectorLengthMismatch(); return result.summary(); }