diff options
author | Anthony Towns <aj@erisian.com.au> | 2019-06-20 18:01:23 +1000 |
---|---|---|
committer | Anthony Towns <aj@erisian.com.au> | 2019-09-10 15:41:50 +1000 |
commit | 3c481f8921bbc587cf287329f39243abe703b868 (patch) | |
tree | 3374bd2d19e1e7c15ac4343e86c3c14450cbae2a /src | |
parent | b5a8d0cff1e5f35e51b5c086e4352cc36f354998 (diff) | |
download | bitcoin-3c481f8921bbc587cf287329f39243abe703b868.tar.xz |
signrawtransactionwithkey: better error messages for bad redeemScript/witnessScript
This adds checks to ensure the redeemScript/witnessScript actually
correspond to the provided scriptPubKey, and, if both are provided,
that they are sensibly related to each other.
Thanks to github user passionofvc for raising this issue.
Diffstat (limited to 'src')
-rw-r--r-- | src/rpc/rawtransaction_util.cpp | 73 |
1 files changed, 57 insertions, 16 deletions
diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 697c6d45c4..e05f28347f 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -196,32 +196,73 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst } // if redeemScript and private keys were given, add redeemScript to the keystore so it can be signed - if (keystore && (scriptPubKey.IsPayToScriptHash() || scriptPubKey.IsPayToWitnessScriptHash())) { + const bool is_p2sh = scriptPubKey.IsPayToScriptHash(); + const bool is_p2wsh = scriptPubKey.IsPayToWitnessScriptHash(); + if (keystore && (is_p2sh || is_p2wsh)) { RPCTypeCheckObj(prevOut, { {"redeemScript", UniValueType(UniValue::VSTR)}, {"witnessScript", UniValueType(UniValue::VSTR)}, }, true); UniValue rs = find_value(prevOut, "redeemScript"); - if (!rs.isNull()) { - std::vector<unsigned char> rsData(ParseHexV(rs, "redeemScript")); - CScript redeemScript(rsData.begin(), rsData.end()); - keystore->AddCScript(redeemScript); - // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). - // This is only for compatibility, it is encouraged to use the explicit witnessScript field instead. - keystore->AddCScript(GetScriptForWitness(redeemScript)); - } UniValue ws = find_value(prevOut, "witnessScript"); - if (!ws.isNull()) { - std::vector<unsigned char> wsData(ParseHexV(ws, "witnessScript")); - CScript witnessScript(wsData.begin(), wsData.end()); - keystore->AddCScript(witnessScript); - // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). - keystore->AddCScript(GetScriptForWitness(witnessScript)); - } if (rs.isNull() && ws.isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript"); } + + // work from witnessScript when possible + std::vector<unsigned char> scriptData(!ws.isNull() ? ParseHexV(ws, "witnessScript") : ParseHexV(rs, "redeemScript")); + CScript script(scriptData.begin(), scriptData.end()); + keystore->AddCScript(script); + // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). + // This is done for redeemScript only for compatibility, it is encouraged to use the explicit witnessScript field instead. + CScript witness_output_script{GetScriptForWitness(script)}; + keystore->AddCScript(witness_output_script); + + if (!ws.isNull() && !rs.isNull()) { + // if both witnessScript and redeemScript are provided, + // they should either be the same (for backwards compat), + // or the redeemScript should be the encoded form of + // the witnessScript (ie, for p2sh-p2wsh) + if (ws.get_str() != rs.get_str()) { + std::vector<unsigned char> redeemScriptData(ParseHexV(rs, "redeemScript")); + CScript redeemScript(redeemScriptData.begin(), redeemScriptData.end()); + if (redeemScript != witness_output_script) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript does not correspond to witnessScript"); + } + } + } + + if (is_p2sh) { + const CTxDestination p2sh{ScriptHash(script)}; + const CTxDestination p2sh_p2wsh{ScriptHash(witness_output_script)}; + if (scriptPubKey == GetScriptForDestination(p2sh)) { + // traditional p2sh; arguably an error if + // we got here with rs.IsNull(), because + // that means the p2sh script was specified + // via witnessScript param, but for now + // we'll just quietly accept it + } else if (scriptPubKey == GetScriptForDestination(p2sh_p2wsh)) { + // p2wsh encoded as p2sh; ideally the witness + // script was specified in the witnessScript + // param, but also support specifying it via + // redeemScript param for backwards compat + // (in which case ws.IsNull() == true) + } else { + // otherwise, can't generate scriptPubKey from + // either script, so we got unusable parameters + throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript/witnessScript does not match scriptPubKey"); + } + } else if (is_p2wsh) { + // plain p2wsh; could throw an error if script + // was specified by redeemScript rather than + // witnessScript (ie, ws.IsNull() == true), but + // accept it for backwards compat + const CTxDestination p2wsh{WitnessV0ScriptHash(script)}; + if (scriptPubKey != GetScriptForDestination(p2wsh)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "redeemScript/witnessScript does not match scriptPubKey"); + } + } } } } |