SEGV in segwit fundrawtransaction #10293

The following will crash bitcoind 0.14.1:

bitcoin-cli -regtest fundrawtransaction 01000000000106220000000000001600149bf98693c5892c3e7b6d991cfeebf146285f59bd00000000 '{"changePosition": 1}'

I've uploaded the corresponding regtest chainstate here: https://github.com/VidaID/bitcoin/blob/fund_segwit_segv/regtest.tar.bz2

Just untar that into ~/.bitcoin, start bitcoind -regtest -daemon=0 and run the above fundrawtransaction.

Stack trace with debug output is:

0x00005555556bde23 in prevector<28u, unsigned char, unsigned int, int>::prevector (this=0x7fff580201f0, other=...)
    at prevector.h:252
252	            new(static_cast<void*>(item_ptr(size() - 1))) T(*it);
(gdb) where
[#0](/bitcoin-bitcoin/0/)  0x00005555556bde23 in prevector<28u, unsigned char, unsigned int, int>::prevector (this=0x7fff580201f0, other=...)
    at prevector.h:252
[#1](/bitcoin-bitcoin/1/)  0x00005555556ba8c2 in CScript::CScript (this=0x7fff580201f0) at ./script/script.h:376
[#2](/bitcoin-bitcoin/2/)  0x00005555556f63cc in CTxOut::CTxOut (this=0x7fff580201e8) at ./primitives/transaction.h:132
[#3](/bitcoin-bitcoin/3/)  0x00005555557c600f in __gnu_cxx::new_allocator<CTxOut>::construct<CTxOut, CTxOut const&> (this=0x7fffb8ff7ea0, 
    __p=0x7fff580201e8, __args#0=...) at /usr/include/c++/6/ext/new_allocator.h:120
[#4](/bitcoin-bitcoin/4/)  0x00005555557c32a6 in std::allocator_traits<std::allocator<CTxOut> >::construct<CTxOut, CTxOut const&> (__a=..., 
    __p=0x7fff580201e8, __args#0=...) at /usr/include/c++/6/bits/alloc_traits.h:455
[#5](/bitcoin-bitcoin/5/)  0x0000555555a87044 in std::vector<CTxOut, std::allocator<CTxOut> >::_M_insert_aux<CTxOut const&> (
    this=0x7fffb8ff7ea0, __position=..., __args#0=...) at /usr/include/c++/6/bits/vector.tcc:361
[#6](/bitcoin-bitcoin/6/)  0x0000555555a7bd69 in std::vector<CTxOut, std::allocator<CTxOut> >::insert (this=0x7fffb8ff7ea0, __position=..., 
    __x=...) at /usr/include/c++/6/bits/vector.tcc:131
[#7](/bitcoin-bitcoin/7/)  0x0000555555a628de in CWallet::FundTransaction (this=0x555556766150, tx=..., nFeeRet=@0x7fffb8ff7e10: 11760, 
    overrideEstimatedFeeRate=false, specificFeeRate=..., nChangePosInOut=@0x7fffb8ff7db8: 1, strFailReason="", 
    includeWatching=false, lockUnspents=false, setSubtractFeeFromOutputs=std::set with 0 elements, keepReserveKey=true, 
    destChange=...) at wallet/wallet.cpp:2344
[#8](/bitcoin-bitcoin/8/)  0x0000555555a27562 in fundrawtransaction (request=...) at wallet/rpcwallet.cpp:2659
[#9](/bitcoin-bitcoin/9/)  0x00005555557d5338 in CRPCTable::execute (this=0x55555611ed00 <tableRPC>, request=...) at rpc/server.cpp:491
[#10](/bitcoin-bitcoin/10/) 0x00005555559476c0 in HTTPReq_JSONRPC (req=0x7fff84001670) at httprpc.cpp:193
[#11](/bitcoin-bitcoin/11/) 0x000055555574c747 in std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), bool (*)(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (__functor=..., 
    __args#0=<unknown type in /usr/local/bin/bitcoind, CU 0x571560, DIE 0x5c6e84>, __args#1="")
    at /usr/include/c++/6/functional:1726
[#12](/bitcoin-bitcoin/12/) 0x0000555555950b7a in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (this=0x7fff84001780, __args#0=0x7fff84001670, __args#1="") at /usr/include/c++/6/functional:2136
[#13](/bitcoin-bitcoin/13/) 0x000055555594ea71 in HTTPWorkItem::operator() (this=0x7fff84001750) at httpserver.cpp:51
[#14](/bitcoin-bitcoin/14/) 0x0000555555951d65 in WorkQueue<HTTPClosure>::Run (this=0x5555561a0050) at httpserver.cpp:132
[#15](/bitcoin-bitcoin/15/) 0x000055555594bb01 in HTTPWorkQueueRun (queue=0x5555561a0050) at httpserver.cpp:361
[#16](/bitcoin-bitcoin/16/) 0x00005555559618f8 in std::_Bind_simple<void (*(WorkQueue<HTTPClosure>*))(WorkQueue<HTTPClosure>*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (this=0x5555561a0248) at /usr/include/c++/6/functional:1400
[#17](/bitcoin-bitcoin/17/) 0x000055555596108c in std::_Bind_simple<void (*(WorkQueue<HTTPClosure>*))(WorkQueue<HTTPClosure>*)>::operator()() (
    this=0x5555561a0248) at /usr/include/c++/6/functional:1389
[#18](/bitcoin-bitcoin/18/) 0x0000555555960a91 in std::thread::_State_impl<std::_Bind_simple<void (*(WorkQueue<HTTPClosure>*))(WorkQueue<HTTPClosure>*)> >::_M_run() (this=0x5555561a0240) at /usr/include/c++/6/thread:196
[#19](/bitcoin-bitcoin/19/) 0x00007ffff615f50f in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
[#20](/bitcoin-bitcoin/20/) 0x00007ffff596d6ca in start_thread (arg=0x7fffb8ff9700) at pthread_create.c:333
[#21](/bitcoin-bitcoin/21/) 0x00007ffff56a70af in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105

It looks like the crash is in dereferencing the iterator *it. other.begin() seems to be pointing to a low memory value 0x06. I'm guessing this is an implementation error in the prevector class.

Dropping the restriction on changeposition or letting it be 0 will not crash.

tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]);

This is being hit with nChangePosInOut == 1 and wtx.tx->vout.size() == 1. So OOB access. Looking into what it should be doing...

nChangePosInOut never gets "unset" in the case of an exact match. It should be set to -1 in that case.

That was fast! Thanks instagibbs!