Merge bitcoin/bitcoin#30444: rest: Reject negative outpoint index early in getutxos parsing

fac932b refactor: Use util::Split to avoid a harmless unsigned-integer-overflow (MarcoFalke)
fab54db 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 fac932b
  hodlinator:
    ut-ACK fac932b
  tdb3:
    re ACK fac932b
  danielabrozzoni:
    ACK fac932b
  brunoerg:
    reACK fac932b

Tree-SHA512: 8f1a75248cb61e1c4beceded6ed170db83b07f30fbcf93a26acfffc00ec4546572366eff87907a7e1423d7d3a2a9e57a0a7a9bacb787c86463f842d7161c16bc

Author: achow101

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)

test/functional/interface_rest.py

@@ -201,10 +201,13 @@ def run_test(self):
         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)])