From 562e3f7b18c7af9076ded6c1ad829aa180515fec Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 10 Mar 2022 10:08:43 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#24371: util: Fix ReadBinaryFile reading beyond maxsize --- src/test/util_tests.cpp | 48 ++++++++++++++++++++++++++++++++++++++ src/util/readwritefile.cpp | 4 ++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 68b6d1519af3a..ea9c73517846b 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include +#include #include #include #include @@ -2747,6 +2749,52 @@ BOOST_AUTO_TEST_CASE(util_ParseByteUnits) BOOST_CHECK(!ParseByteUnits("1x", noop)); } +BOOST_AUTO_TEST_CASE(util_ReadBinaryFile) +{ + fs::path tmpfolder = m_args.GetDataDirBase(); + fs::path tmpfile = tmpfolder / "read_binary.dat"; + std::string expected_text; + for (int i = 0; i < 30; i++) { + expected_text += "0123456789"; + } + { + std::ofstream file{tmpfile}; + file << expected_text; + } + { + // read all contents in file + auto [valid, text] = ReadBinaryFile(tmpfile); + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(text, expected_text); + } + { + // read half contents in file + auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2); + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(text, expected_text.substr(0, expected_text.size() / 2)); + } + { + // read from non-existent file + fs::path invalid_file = tmpfolder / "invalid_binary.dat"; + auto [valid, text] = ReadBinaryFile(invalid_file); + BOOST_CHECK(!valid); + BOOST_CHECK(text.empty()); + } +} + +BOOST_AUTO_TEST_CASE(util_WriteBinaryFile) +{ + fs::path tmpfolder = m_args.GetDataDirBase(); + fs::path tmpfile = tmpfolder / "write_binary.dat"; + std::string expected_text = "bitcoin"; + auto valid = WriteBinaryFile(tmpfile, expected_text); + std::string actual_text; + std::ifstream file{tmpfile}; + file >> actual_text; + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(actual_text, expected_text); +} + BOOST_AUTO_TEST_CASE(clearshrink_test) { { diff --git a/src/util/readwritefile.cpp b/src/util/readwritefile.cpp index 2493028f95430..7bf0c7dd8a072 100644 --- a/src/util/readwritefile.cpp +++ b/src/util/readwritefile.cpp @@ -20,7 +20,7 @@ std::pair ReadBinaryFile(const fs::path &filename, size_t maxs std::string retval; char buffer[128]; do { - const size_t n = fread(buffer, 1, sizeof(buffer), f); + const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - retval.size()), f); // Check for reading errors so we don't return any data if we couldn't // read the entire file (or up to maxsize) if (ferror(f)) { @@ -28,7 +28,7 @@ std::pair ReadBinaryFile(const fs::path &filename, size_t maxs return std::make_pair(false,""); } retval.append(buffer, buffer+n); - } while (!feof(f) && retval.size() <= maxsize); + } while (!feof(f) && retval.size() < maxsize); fclose(f); return std::make_pair(true,retval); }