Soundness problem due to exception safety violation in lock design

[Evian-Zhang]

Hello, we are security researchers targeting Rust security. By running our tool in this repository, we found a soundness issue. This report is written by 100% human. We promise that all you read will never be generated by LLM.

Proof of concept

static mut FOO: u8 = 0;

fn main() {
    // Pure safe code below
    let _ = std::panic::catch_unwind(|| {
        backtrace::resolve((&raw mut FOO).cast(), |_symbol| {
            panic!("poison the mutex");
        });
    });
    let _ = std::panic::catch_unwind(|| {
        backtrace::resolve((&raw mut FOO).cast(), |_| {});
    });
    // Soundness issue happens here
}

Explanation

In backtrace::resolve, crate::lock::lock() is called to get a mutex guard of global lock as here:

backtrace-rs/src/lib.rs

Lines 217 to 229 in 06b2cc5

	pub fn lock() -> LockGuard {
	// If we're the thread holding this lock, pretend to acquire the lock
	// again by returning a LockGuard(None)
	if LOCK_HELD.with(|l| l.get()) {
	return LockGuard(None);
	}
	// Insist that we totally are the thread holding the lock
	// (our thread will block until we are)
	LOCK_HELD.with(|s| s.set(true));
	// ok *actually* try to acquire the lock, blocking as necessary
	LockGuard(Some(LOCK.lock().unwrap()))
	}
	}

And in the drop implementation of LockGuard, the MutexGuard is simply dropped without cleaning the poison:

backtrace-rs/src/lib.rs

Lines 152 to 165 in 06b2cc5

	impl Drop for LockGuard {
	fn drop(&mut self) {
	// Don't do anything if we're a LockGuard(None)
	if self.0.is_some() {
	LOCK_HELD.with(|slot| {
	// Immediately crash if we somehow aren't the thread holding this lock
	assert!(slot.get());
	// We are no longer the thread holding this lock
	slot.set(false);
	});
	}
	// lock implicitly released here, if we're a LockGuard(Some(..))
	}
	}

As a result, in above PoC, after the first backtrace::resolve, the global LOCK is actually poisoned. So in the second backtrace::resolve, the crate::lock::lock will panic at unwraping in the LOCK.lock().unwrap(). However, we caught the panic again by std::panic::catch_unwind, so after the second catch_unwind, the LOCK_HELD is true (the cleaning function will never be invoked).

Then, every call to any function in the library that requires the lock will be unguarded (as the lock will never be locked if LOCK_HELD is true), leading to race conditions.

[workingjubilee]

This is #758 no?

[Evian-Zhang]

@workingjubilee No, I don't think so. Exception safety violation is special that this kind of vulnerability can hardly share the same root cause with other vulns in normal execution paths. Moreover, that issue points out a UAF bug related to mmap in gimli module, while this issue is related to race conditions in crate::lock, which is used widely across this crate.