utACK 71a6133

MutateTxAddOutPubKey appears to have the same issue. I recommend reviewing 61a1534 and addressing it completely.

Also, pinging @jnewbery

Ugh. Needs backport to 0.14.1

I've reviewed 61a1534 and I think MutateTxAddOutAddr() and MutateTxAddOutPubKey() are the only functions that aren't properly checking their inputs. Extra eyes to review are always good though.

The check in MutateTxAddOutPubKey() should be:

    if (vStrInputParts.size() < 2 || vStrInputParts.size() > 3)
        throw std::runtime_error("TX output missing or too many separators");

since MutateTxAddOutPubKey() can have an optional flag.

I'm currently writing test cases for bitcoin-tx that test this and other errors. We should include that in this PR when it's ready.

Extra testcases are here: https://github.com/jnewbery/bitcoin/commits/bitcoin_tx_input_sanitiser

• 31c4dbf81a164e57a414ee548ec3227dfee35cba adds stderr checking to bctest.py
• 718063a84dd78187171f60759bebd64aea1fed58 is the fix for bitcoin-tx (which you should just squash into your existing fix)
• 0768e51977924aadf19c21331a15e30d97ba44c0 adds tests for bitcoin-tx correctly checking the number of separators in inputs.

Yes, it seems MutateTxAddOutAddr and MutateTxAddOutPubKey are the only ones with more than one part but not checking it.
utACK but better to fix both.

@awemany - there's some enthusiasm to get this merged into master quickly, so I've opened #10130 with your fix and my test changes.

Thanks for reporting this (and providing a fix!)

Closing in favour of #10130 . @awemany your changes are still included in the new pull-request.