diff options
author | Ava Chow <github@achow101.com> | 2024-07-18 16:51:42 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-07-18 16:51:42 -0400 |
commit | 6144aa21d0606e66ed6635975698e9a6456b5ca3 (patch) | |
tree | 8a20726152baddcab4c2aafea97b78471adf6932 | |
parent | 20ccb30b7af1c30cece2b0579ba88aa101583f07 (diff) | |
parent | fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 (diff) | |
download | bitcoin-6144aa21d0606e66ed6635975698e9a6456b5ca3.tar.xz |
Merge bitcoin/bitcoin#30444: rest: Reject negative outpoint index early in getutxos parsing
fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 refactor: Use util::Split to avoid a harmless unsigned-integer-overflow (MarcoFalke)
fab54db9f1d0e634f4a697480dbb87b87940dc5c rest: Reject negative outpoint index in getutxos parsing (MarcoFalke)
Pull request description:
In `rest_getutxos` outpoint indexes such as `+N` or `-N` are accepted. This should be harmless, because any index out of range should be treated as a non-existent utxo. However, a negative index can't exist ever, so it seems better to reject all signs, whether `+` or `-`.
ACKs for top commit:
achow101:
ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
hodlinator:
ut-ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
tdb3:
re ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
danielabrozzoni:
ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
brunoerg:
reACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
Tree-SHA512: 8f1a75248cb61e1c4beceded6ed170db83b07f30fbcf93a26acfffc00ec4546572366eff87907a7e1423d7d3a2a9e57a0a7a9bacb787c86463f842d7161c16bc
-rw-r--r-- | src/rest.cpp | 13 | ||||
-rwxr-xr-x | test/functional/interface_rest.py | 5 |
2 files changed, 12 insertions, 6 deletions
diff --git a/src/rest.cpp b/src/rest.cpp index 4abbc4d2ca..3cf6ad343c 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -788,14 +788,17 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: for (size_t i = (fCheckMemPool) ? 1 : 0; i < uriParts.size(); i++) { - int32_t nOutput; - std::string strTxid = uriParts[i].substr(0, uriParts[i].find('-')); - std::string strOutput = uriParts[i].substr(uriParts[i].find('-')+1); + const auto txid_out{util::Split<std::string_view>(uriParts[i], '-')}; + if (txid_out.size() != 2) { + return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); + } + auto output{ToIntegral<uint32_t>(txid_out.at(1))}; - if (!ParseInt32(strOutput, &nOutput) || !IsHex(strTxid)) + if (!output || !IsHex(txid_out.at(0))) { return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); + } - vOutPoints.emplace_back(TxidFromString(strTxid), (uint32_t)nOutput); + vOutPoints.emplace_back(TxidFromString(txid_out.at(0)), *output); } if (vOutPoints.size() > 0) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index ae8d6b226d..aa201cb85b 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -201,10 +201,13 @@ class RESTTest (BitcoinTestFramework): json_obj = self.test_rest_request(f"/getutxos/checkmempool/{spending[0]}-{spending[1]}") assert_equal(len(json_obj['utxos']), 1) - # Do some invalid requests + self.log.info("Check some invalid requests") self.test_rest_request("/getutxos", http_method='POST', req_type=ReqType.JSON, body='{"checkmempool', status=400, ret_type=RetType.OBJ) self.test_rest_request("/getutxos", http_method='POST', req_type=ReqType.BIN, body='{"checkmempool', status=400, ret_type=RetType.OBJ) self.test_rest_request("/getutxos/checkmempool", http_method='POST', req_type=ReqType.JSON, status=400, ret_type=RetType.OBJ) + self.test_rest_request(f"/getutxos/{spending[0]}_+1", ret_type=RetType.OBJ, status=400) + self.test_rest_request(f"/getutxos/{spending[0]}-+1", ret_type=RetType.OBJ, status=400) + self.test_rest_request(f"/getutxos/{spending[0]}--1", ret_type=RetType.OBJ, status=400) # Test limits long_uri = '/'.join([f"{txid}-{n_}" for n_ in range(20)]) |