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

Remote segfault in gossip #8873

Closed
leoluk opened this issue Mar 15, 2020 · 5 comments
Closed

Remote segfault in gossip #8873

leoluk opened this issue Mar 15, 2020 · 5 comments
Assignees
Labels
security Pull requests that address a security vulnerability
Milestone

Comments

@leoluk
Copy link
Contributor

leoluk commented Mar 15, 2020

Problem

Sending this packet to the gossip port segfaults solana-validator:

cat > /dev/udp/127.0.0.1/gossip-port < 844217987-8152.txt

844217987-8152.txt

Proposed Solution

Dunno

@mvines mvines added this to the v1.0.7 milestone Mar 15, 2020
@mvines
Copy link
Member

mvines commented Mar 15, 2020

Thread 13 "solana-listen" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeddff700 (LWP 56357)]
0x00005555556c3311 in solana_core::crds_gossip_pull::CrdsGossipPull::process_pull_requests ()
(gdb) bt
#0  0x00005555556c3311 in solana_core::crds_gossip_pull::CrdsGossipPull::process_pull_requests ()
#1  0x0000555555755ae0 in core::ops::function::impls::<impl core::ops::function::Fn<A> for &F>::call ()
#2  0x00005555557ddb41 in <core::iter::adapters::Map<I,F> as core::iter::traits::iterator::Iterator>::fold ()
#3  0x0000555555838005 in rayon::iter::plumbing::bridge_producer_consumer::helper ()
#4  0x00005555558200d9 in <rayon::iter::plumbing::bridge::Callback<C> as rayon::iter::plumbing::ProducerCallback<I>>::callback ()
#5  0x00005555556ebafa in rayon::iter::from_par_iter::<impl rayon::iter::FromParallelIterator<()> for ()>::from_par_iter ()
#6  0x00005555556c92f9 in <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once ()
#7  0x0000555555755097 in std::panicking::try::do_call ()
#8  0x00005555562cba0a in __rust_maybe_catch_panic () at src/libpanic_unwind/lib.rs:78
#9  0x00005555556f621d in <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute ()
#10 0x0000555555deb02e in rayon_core::registry::WorkerThread::wait_until_cold ()
#11 0x0000555555de97ea in rayon_core::registry::ThreadBuilder::run ()
#12 0x0000555555debfe1 in std::sys_common::backtrace::__rust_begin_short_backtrace ()
#13 0x0000555555deb370 in std::panicking::try::do_call ()
#14 0x00005555562cba0a in __rust_maybe_catch_panic () at src/libpanic_unwind/lib.rs:78
#15 0x0000555555dee337 in core::ops::function::FnOnce::call_once{{vtable-shim}} ()
#16 0x00005555562accaf in <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once () at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/liballoc/boxed.rs:1022
#17 0x00005555562caad0 in <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once () at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/liballoc/boxed.rs:1022
#18 std::sys_common::thread::start_thread () at src/libstd/sys_common/thread.rs:13
#19 std::sys::unix::thread::Thread::new::thread_start () at src/libstd/sys/unix/thread.rs:80
#20 0x00007ffff76306db in start_thread (arg=0x7fffeddff700) at pthread_create.c:463
#21 0x00007ffff69e388f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

@mvines
Copy link
Member

mvines commented Mar 15, 2020

https://crates.io/crates/bv is full of unsafe, https://github.com/tov/bv-rs/search?q=unsafe&type=

844217987-8152.txt generates a CrdsFilter with bits.len() of 18014398509488747

pub bits: BitVec<u64>,

which blows up in here:

if !filter.contains(&v.value_hash) {

at
if !self.bits.get(pos) {

because pos = 17758251168553335

@mvines
Copy link
Member

mvines commented Mar 15, 2020

We might be able to apply some limited_deserialize() on our Bloom struct to ensure the BitVec length is proper before allowing the BitVec implementation to perform its unsafe memory operations:

#[derive(Serialize, Deserialize, Default, Clone, Debug, PartialEq)]
pub struct Bloom<T: BloomHashIndex> {
pub keys: Vec<u64>,
pub bits: BitVec<u64>,
num_bits_set: u64,
_phantom: PhantomData<T>,
}

An upstream fix in BitVec is probably a better longer-term solution

@mvines
Copy link
Member

mvines commented Mar 19, 2020

Fix coming in via #8952

@ryoqun
Copy link
Member

ryoqun commented Mar 19, 2020

Just to be sure, I've confirmed the vulnerability no longer exists after #8955. So closing now.

@ryoqun ryoqun closed this as completed Mar 19, 2020
@leoluk leoluk added the security Pull requests that address a security vulnerability label Sep 16, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
security Pull requests that address a security vulnerability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants