Undo broken invariant before panicking in ref count overflow

[CosmicHorrorDev]

The last PR in today's barrage is an extra spooky one :ghost::ghost::ghost:

I believe this should be enough to fix exception safety from overflowing the ref count. This prevents a possible use-after-free that can occur in incredibly degenerate programs. For example

use ecow::EcoString;

fn main() {
    let one = EcoString::from("This is so large that it's a `Repr::Large`");
    let two = one.clone();

    for _ in 0..(usize::MAX - 1) {
        let _ = std::panic::catch_unwind(|| {
            std::mem::forget(two.clone());
        });
    }

    // The ref count is now at 2 + usize::MAX - 1 aka we overflowed to 1 :D

    drop(two);

    // Now that the ref count is 0 the backing allocation gets dropped, buuuuut we still have a
    // reference to it which lets us UAF

    println!("Boom {one}");
}

Considering this involves overflowing a usize it takes a considerable amount of time (I tried optimizing things, but even using cross to run on a 32-bit platform gets really slow when we start catching panics)

If you wind up adding an std feature you could change ref_count_overflow() to abort when the feature is enabled which matches the standard library's handling

[laurmaedje]

Good catch! I had abort first, then made the crate no_std, didn't see a way to abort and just gave up and panicked instead. Is there a particular reason why you take the roundtrip through drop instead of just decreasing the ref-count before panicking?

[CosmicHorrorDev]

I kinda figured it was something along those lines

Is there a particular reason why you take the roundtrip through drop instead of just decreasing the ref-count before panicking?

Not really a major one :sweat_smile:

I mostly just wanted to avoid spreading out where different atomics are used to make it easier to keep things in sync. If you want I can switch to just directly decreasing the ref count

[laurmaedje]

Makes sense. Thanks for the fix!