Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code vulnerable to segfault after a network fork #5698

Closed
eduffield222 opened this issue Jan 23, 2015 · 4 comments
Closed

Code vulnerable to segfault after a network fork #5698

eduffield222 opened this issue Jan 23, 2015 · 4 comments

Comments

@eduffield222
Copy link

Hello,

I'm Evan Duffield, the lead developer of Darkcoin. We recently re-forked from Bitcoin 0.9.3 away from litecoin and after which we had a minor network fork, with 2 valid chains for a period of time. This lead many clients getting segfaults, after hitting this line of code:

https://github.com/bitcoin/bitcoin/blob/0.10/src/main.cpp#L1274

I've looked through updates done to Bitcoin since 0.9.3 and this looks like an unknown issue in the Bitcoin codebase. This is important and must get fixed, because a good portion of the network is on vulnerable versions and my understanding is they'll all segfault when a fork condition is met.

Here's the stack trace from Darkcoin.

darkcoind 0.11.0.11

(gdb) bt
#0 0x000000000046a83e in ToString (this=<error reading variable: Cannot access memory at address 0x0>) at uint256.h:343
#1 CheckForkWarningConditions () at main.cpp:1749
#2 0x000000000047e3d5 in AddToBlockIndex (block=..., state=..., pos=...) at main.cpp:2734
#3 0x000000000047e9d0 in AcceptBlock (block=..., state=..., dbp=dbp@entry=0x0) at main.cpp:3065
#4 0x000000000047f991 in ProcessBlock (state=..., pfrom=pfrom@entry=0xb6ed40, pblock=pblock@entry=0x7fffdeffc460, dbp=dbp@entry=0x0) at main.cpp:3184
#5 0x000000000048475f in ProcessMessage (pfrom=pfrom@entry=0xb6ed40, strCommand=..., vRecv=...) at main.cpp:4468
#6 0x0000000000486b53 in ProcessMessages (pfrom=0xb6ed40) at main.cpp:4780
#7 0x000000000050a71f in operator() (a0=, this=) at /usr/include/boost/function/function_template.hpp:760
#8 m_invoke (connectionBody=..., this=0x7fffdeffca60) at /usr/include/boost/signals2/detail/signal_template.hpp:368
#9 operator() (connectionBody=..., this=0x7fffdeffca60) at /usr/include/boost/signals2/detail/signal_template.hpp:345
#10 dereference (this=) at /usr/include/boost/signals2/detail/slot_call_iterator.hpp:82
#11 dereference<boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::signal1_impl<bool, CNode*, boost::signals2::optional_last_value, int, std::less, boost::function<bool(CNode*)>, boost::function<bool(const boost::signals2::connection&, CNode*)>, boost::signals2::mutex>::slot_invoker, std::_List_iterator<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > >, boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > > (f=) at /usr/include/boost/iterator/iterator_facade.hpp:517
#12 operator* (this=) at /usr/include/boost/iterator/iterator_facade.hpp:643
#13 operator()<boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::signal1_impl<bool, CNode*, boost::signals2::optional_last_value, int, std::less, boost::function<bool(CNode*)>, boost::function<bool(const boost::signals2::connection&, CNode*)>, boost::signals2::mutex>::slot_invoker, std::_List_iterator<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > >, boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > > (first=..., this=, last=...) at /usr/include/boost/signals2/optional_last_value.hpp:34
#14 operator()boost::signals2::optional_last_value<bool, boost::signals2::detail::slot_call_iterator_t<boost::signals2::detail::signal1_impl<bool, CNode*, boost::signals2::optional_last_value, int, std::less, boost::function<bool(CNode*)>, boost::function<bool(const boost::signals2::connection&, CNode*)>, boost::signals2::mutex>::slot_invoker, std::_List_iterator<boost::shared_ptr<boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > >, boost::signals2::detail::connection_body<std::pair<boost::signals2::detail::slot_meta_group, boost::optional >, boost::signals2::slot1<bool, CNode*, boost::function<bool(CNode*)> >, boost::signals2::mutex> > > (first=..., this=, combiner=..., last=...) at /usr/include/boost/signals2/detail/result_type_wrapper.hpp:53
#15 boost::signals2::detail::signal1_impl<bool, CNode*, boost::signals2::optional_last_value, int, std::less, boost::function<bool (CNode*)>, boost::function<bool (boost::signals2::connection const&, CNode*)>, boost::signals2::mutex>::operator()(CNode*) (this=, arg1=arg1@entry=0xb6ed40) at /usr/include/boost/signals2/detail/signal_template.hpp:246
#16 0x00000000004f0155 in operator() (arg1=0xb6ed40, this=0xb240d8) at /usr/include/boost/signals2/detail/signal_template.hpp:695
#17 ThreadMessageHandler () at net.cpp:1529
#18 0x00000000004f7bee in TraceThread<void (*)()> (name=0x74f2d1 "msghand", func=0x4effa0 <ThreadMessageHandler()>) at util.h:575
#19 0x00007ffff7f55629 in ?? () from /usr/lib/libboost_thread.so.1.49.0
#20 0x00007ffff6839b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#21 0x00007ffff65837bd in clone () from /lib/x86_64-linux-gnu/libc.so.6
#22 0x0000000000000000 in ?? ()

@eduffield222
Copy link
Author

Notice the segfault is hitting pindexBestForkBase->phashBlock and trying to turn that into a String, the vulnerability still is present is 0.9.4 and 0.10.

#0 0x000000000046a83e in ToString (this=) at uint256.h:343
#1 CheckForkWarningConditions () at main.cpp:1749

dashpay@90ad641

@gmaxwell
Copy link
Contributor

I believe you've misunderstood the behavior. No block can be in the block index there without a hash, if one is there is an error elsewhere. See main.cpp: AddToBlockIndex. (Also-- I'm unable to reproduce, the warnings act fine.)

As an aside your response of posting something you believed to be a "vulnerability" on reddit instead of reporting it to the security contact is irresponsible.

@eduffield222
Copy link
Author

I didn't submit it to reddit all, I just made an issue and a pull request. I didn't believe this was incredibly serious, due to requiring a 12 hour long fork to cause issues on the Bitcoin network. However, there's no checks at all in 0.9, so regardless of how you patch it, it needs to get patched. Thanks.

@sipa
Copy link
Member

sipa commented Jan 24, 2015

Feel free to keep reporting problems you find, nonetheless!

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants