diff options
author | MarcoFalke <falke.marco@gmail.com> | 2019-12-16 10:15:51 -0500 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2019-12-16 10:17:17 -0500 |
commit | 6b51cce65afd9a6357277c4339d7cf56814e476e (patch) | |
tree | 0455fe615a2033d89cc1f2608869aaeb4155919f | |
parent | 5e4912f99096e9955b332df004662f83885b322f (diff) | |
parent | 137c80d579502e329964d7d1028a9507d4667774 (diff) |
Merge #17753: util: Don't allow Base32/64-decoding or ParseMoney(…) on strings with embedded NUL characters. Add tests.
137c80d579502e329964d7d1028a9507d4667774 tests: Add tests for decoding/parsing of base32, base64 and money strings containing NUL characters (practicalswift)
a6fc26da55dea3b76bd89fbbca24ded170238674 util: Don't allow DecodeBase32(...) of strings with embedded NUL characters (practicalswift)
93cc18b0f6fa5fa8144079a4f51904d8b3087e94 util: Don't allow DecodeBase64(...) of strings with embedded NUL characters (practicalswift)
ccc53e43c5464058171d6291da861a88184b230e util: Don't allow ParseMoney(...) of strings with embedded NUL characters (practicalswift)
Pull request description:
Don't allow Base32/64-decoding or `ParseMoney(…)` on strings with embedded `NUL` characters. Add tests.
Added tests before:
```
$ src/test/test_bitcoin
Running 385 test cases...
test/base32_tests.cpp(31): error: in "base32_tests/base32_testvectors":
check failure == true has failed [false != true]
test/base64_tests.cpp(31): error: in "base64_tests/base64_testvectors":
check failure == true has failed [false != true]
test/util_tests.cpp(1074): error: in "util_tests/util_ParseMoney":
check !ParseMoney(std::string("\0-1", 3), ret) has failed
test/util_tests.cpp(1076): error: in "util_tests/util_ParseMoney":
check !ParseMoney(std::string("1\0", 2), ret) has failed
*** 4 failures are detected in the test module "Bitcoin Core Test Suite"
```
Added tests after:
```
$ src/test/test_bitcoin
Running 385 test cases...
*** No errors detected
```
ACKs for top commit:
laanwj:
Code review ACK 137c80d579502e329964d7d1028a9507d4667774
Tree-SHA512: 9486a0d32b4cf686bf5a47a0778338ac571fa39c66ad6d6d6cede58ec798e87bb50a2f9b7fd79ecd1fef1ba284e4073c1b430110967073ff87bdbbde7cada447
-rw-r--r-- | src/test/base32_tests.cpp | 11 | ||||
-rw-r--r-- | src/test/base64_tests.cpp | 11 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 5 | ||||
-rw-r--r-- | src/util/moneystr.cpp | 4 | ||||
-rw-r--r-- | src/util/strencodings.cpp | 12 |
5 files changed, 43 insertions, 0 deletions
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) 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 <tinyformat.h> #include <util/strencodings.h> +#include <util/string.h> 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); } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 8f2d05f03b..31719cd975 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -191,6 +191,12 @@ std::vector<unsigned char> 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<unsigned char> vchRet = DecodeBase64(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } @@ -260,6 +266,12 @@ std::vector<unsigned char> 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<unsigned char> vchRet = DecodeBase32(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } |