From ccc53e43c5464058171d6291da861a88184b230e Mon Sep 17 00:00:00 2001 From: practicalswift Date: Mon, 16 Dec 2019 08:59:48 +0000 Subject: [PATCH 1/4] util: Don't allow ParseMoney(...) of strings with embedded NUL characters --- src/util/moneystr.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp index ba5a12e58c..3e75a2e3e9 100644 --- a/src/util/moneystr.cpp +++ b/src/util/moneystr.cpp @@ -7,6 +7,7 @@ #include #include +#include std::string FormatMoney(const CAmount& n) { @@ -32,6 +33,9 @@ std::string FormatMoney(const CAmount& n) bool ParseMoney(const std::string& str, CAmount& nRet) { + if (!ValidAsCString(str)) { + return false; + } return ParseMoney(str.c_str(), nRet); } From 93cc18b0f6fa5fa8144079a4f51904d8b3087e94 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Mon, 16 Dec 2019 09:00:08 +0000 Subject: [PATCH 2/4] util: Don't allow DecodeBase64(...) of strings with embedded NUL characters --- src/util/strencodings.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 8f2d05f03b..dfd8961cb7 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -191,6 +191,12 @@ std::vector DecodeBase64(const char* p, bool* pf_invalid) std::string DecodeBase64(const std::string& str, bool* pf_invalid) { + if (!ValidAsCString(str)) { + if (pf_invalid) { + *pf_invalid = true; + } + return {}; + } std::vector vchRet = DecodeBase64(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } From a6fc26da55dea3b76bd89fbbca24ded170238674 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Mon, 16 Dec 2019 09:00:20 +0000 Subject: [PATCH 3/4] util: Don't allow DecodeBase32(...) of strings with embedded NUL characters --- src/util/strencodings.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index dfd8961cb7..31719cd975 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -266,6 +266,12 @@ std::vector DecodeBase32(const char* p, bool* pf_invalid) std::string DecodeBase32(const std::string& str, bool* pf_invalid) { + if (!ValidAsCString(str)) { + if (pf_invalid) { + *pf_invalid = true; + } + return {}; + } std::vector vchRet = DecodeBase32(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } From 137c80d579502e329964d7d1028a9507d4667774 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Mon, 16 Dec 2019 09:09:17 +0000 Subject: [PATCH 4/4] tests: Add tests for decoding/parsing of base32, base64 and money strings containing NUL characters --- src/test/base32_tests.cpp | 11 +++++++++++ src/test/base64_tests.cpp | 11 +++++++++++ src/test/util_tests.cpp | 5 +++++ 3 files changed, 27 insertions(+) diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index bd6ece935b..690368b177 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) std::string strDec = DecodeBase32(vstrOut[i]); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } + + // Decoding strings with embedded NUL characters should fail + bool failure; + (void)DecodeBase32(std::string("invalid", 7), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase32(std::string("AWSX3VPP", 8), &failure); + BOOST_CHECK_EQUAL(failure, false); + (void)DecodeBase32(std::string("AWSX3VPP\0invalid", 16), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase32(std::string("AWSX3VPPinvalid", 15), &failure); + BOOST_CHECK_EQUAL(failure, true); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index a5fed55504..94df4d1955 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) std::string strDec = DecodeBase64(strEnc); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } + + // Decoding strings with embedded NUL characters should fail + bool failure; + (void)DecodeBase64(std::string("invalid", 7), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase64(std::string("nQB/pZw=", 8), &failure); + BOOST_CHECK_EQUAL(failure, false); + (void)DecodeBase64(std::string("nQB/pZw=\0invalid", 16), &failure); + BOOST_CHECK_EQUAL(failure, true); + (void)DecodeBase64(std::string("nQB/pZw=invalid", 15), &failure); + BOOST_CHECK_EQUAL(failure, true); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b9fcd97a8f..a5cbae89b4 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1069,6 +1069,11 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) // Parsing negative amounts must fail BOOST_CHECK(!ParseMoney("-1", ret)); + + // Parsing strings with embedded NUL characters should fail + BOOST_CHECK(!ParseMoney(std::string("\0-1", 3), ret)); + BOOST_CHECK(!ParseMoney(std::string("\01", 2), ret)); + BOOST_CHECK(!ParseMoney(std::string("1\0", 2), ret)); } BOOST_AUTO_TEST_CASE(util_IsHex)