Skip to content

Add advisory for rkyv: panic safety in InlineVec::clear and SerVec::clear#2815

Open
tooson9010-spec wants to merge 10 commits intorustsec:mainfrom
tooson9010-spec:rkyv-panic-safety-advisory
Open

Add advisory for rkyv: panic safety in InlineVec::clear and SerVec::clear#2815
tooson9010-spec wants to merge 10 commits intorustsec:mainfrom
tooson9010-spec:rkyv-panic-safety-advisory

Conversation

@tooson9010-spec
Copy link
Copy Markdown

This advisory covers two panic-safety bugs in rkyv v0.8.15:

  • InlineVec::clear() (src/util/inline_vec.rs)
  • SerVec::clear() (src/util/ser_vec.rs)

Both functions update self.len only after drop_in_place. If an element's Drop panics, len is left stale, and a subsequent invocation of clear() (triggered by InlineVec::drop or SerVec::with_capacity, respectively) re-visits already-freed memory. Both are reachable from safe Rust and result in double-free / heap-use-after-free, confirmed by AddressSanitizer and Miri.

Disclosure

The maintainer (David Koloski) was notified privately by email and acknowledged the report. Both issues were fixed in v0.8.16. The maintainer also explicitly granted permission to disclose the issues publicly:

"This is a bug, but I think this is not a critical security vulnerability and you have permission to disclose it wherever you see fit. I patched the two issues you mentioned before I released version 0.8.16 just now."

Comment thread crates/rkyv/RUSTSEC-0000-0000.md
@djc
Copy link
Copy Markdown
Member

djc commented Apr 23, 2026

I think this is not a critical security vulnerability

I'd like to better understand why this is -- maybe this doesn't need to be an advisory?

@tooson9010-spec
Copy link
Copy Markdown
Author

@djc Thank you for raising this — it's a fair question, and I defer to your judgment on where the threshold should be. Let me briefly explain my reasoning, in case it's useful:

My main reason for filing was that reaching memory unsafety from safe Rust is a soundness violation, which RustSec has historically treated as in-scope regardless of exploitability. I agree the CVSS is Medium at most (local, high AC, mostly availability), and I wasn't trying to frame it as critical — the maintainer's "not critical" wording matches my own assessment. One small nuance on reachability: catch_unwind isn't always written by the user directly; it's present inside several common frameworks (request handlers, task runtimes), so consumers can end up on that path transitively.

That said, if this falls below the RustSec bar in your view, I'm very happy to close the PR — the maintainer's GHSA will still cover GitHub dependency alerts. Either way, thanks for taking the time to look, and your read here will help me calibrate for future reports.

@djc
Copy link
Copy Markdown
Member

djc commented Apr 23, 2026

My main reason for filing was that reaching memory unsafety from safe Rust is a soundness violation, which RustSec has historically treated as in-scope regardless of exploitability. I agree the CVSS is Medium at most (local, high AC, mostly availability), and I wasn't trying to frame it as critical — the maintainer's "not critical" wording matches my own assessment. One small nuance on reachability: catch_unwind isn't always written by the user directly; it's present inside several common frameworks (request handlers, task runtimes), so consumers can end up on that path transitively.

Yes, I'm trying to recalibrate what we consider in scope, see:

(which as you'll note was written in response to earlier feedback from the rkyv maintainer).

In this case, while the behavior is unsound, the setup conditions for this seem pretty hard to reach by accident -- catching a panic, then panicking in Drop outside the library, so I think this might not merit an advisory.

@tooson9010-spec
Copy link
Copy Markdown
Author

@djc That's very helpful context — thanks for pointing me to #2572. I agree the setup conditions here are hard to reach by accident, and I'll close this PR. The maintainer's GHSA will cover downstream alerts, which is likely sufficient for a bug of this shape.

I'll take the criteria in #2572 into account going forward — much appreciate the thoughtful review.

@8573
Copy link
Copy Markdown
Contributor

8573 commented Apr 23, 2026

Both are reachable from safe Rust and result in double-free / heap-use-after-free

As far as I see, the RustSec maintainers who commented in #2572 agreed that, although panics are not worthy of advisories unless a crate's documentation says so, unsoundness is worthy of an advisory (with informational = "unsound"). Is that not so?

That said, something that triggers UB seems like it'll always quality for a advisory.

Agreed that triggering UB should always be in scope.

Maybe this could be added only once cargo-deny and GitHub (github/advisory-database#7418) stop surfacing informational advisories without user opt-in.

@tooson9010-spec
Copy link
Copy Markdown
Author

tooson9010-spec commented Apr 24, 2026

Thanks again for all the patient guidance on this. I wanted to ask one more clarifying question, purely to calibrate my own understanding for future reports — please tell me if I'm missing something obvious.

I noticed that RUSTSEC-2026-0103 was published just a few days ago (April 21), covering essentially the same pattern in thin-vec:

  • Same root cause: metadata update (set_len(0)) placed after drop_in_place rather than before, leading to double-free / UAF when an element's Drop panics.
  • Same affected-API shape: IntoIter::drop and clear() — structurally identical to InlineVec::clear / SerVec::clear here.
  • Same reachability profile: safe Rust, catch_unwind in the PoC, Drop panic as the trigger.
  • Confirmed the same way: AddressSanitizer + Miri.

That one was accepted as a regular advisory (CVSS 7.3 High, CVE-2026-6654), not informational = "unsound".

I don't want to relitigate the decision here — I'll follow whatever you think is right for this PR. But I'm genuinely trying to understand whether the distinction is about something specific to rkyv (e.g. less heap-owning usage in practice, or a different typical deployment shape), or whether the recalibration in #2572 is still in flux and RUSTSEC-2026-0103 was merged before the new bar was applied. Any pointer you can give would really help me calibrate for future disclosures — thanks again.

@djc
Copy link
Copy Markdown
Member

djc commented Apr 24, 2026

I don't want to relitigate the decision here — I'll follow whatever you think is right for this PR. But I'm genuinely trying to understand whether the distinction is about something specific to rkyv (e.g. less heap-owning usage in practice, or a different typical deployment shape), or whether the recalibration in #2572 is still in flux and RUSTSEC-2026-0103 was merged before the new bar was applied. Any pointer you can give would really help me calibrate for future disclosures — thanks again.

My reviews are definitely not perfect, but I do think a material differerence here is that the thin-vec advisory affects what I'd consider the primary API -- implementations directly accessible via thin_vec::Vec. On the other hand, it's not obvious to me how often InlineVec and SerVec are likely to be exposed to "user" code, as in, code that is not generated from rkyv macros.

@tooson9010-spec
Copy link
Copy Markdown
Author

@djc I wanted to update you on additional research that significantly impacts
the assessment of these vulnerabilities.

Advanced analysis has confirmed that both rkyv issues can be exploited to
achieve code execution, moving the impact classification from memory safety
violations to demonstrated security compromise.

I'd prefer to discuss the technical specifics privately, but I can confirm
this research establishes the high-impact security justification that
addresses your signal/noise ratio concerns.

Would you be willing to reconsider the advisory based on confirmed code
execution capability?

@djc
Copy link
Copy Markdown
Member

djc commented Apr 24, 2026

Yes, that sounds right.

@tooson9010-spec
Copy link
Copy Markdown
Author

@djc Thank you! Just to confirm - does this mean you're now supportive
of a RustSec advisory for the rkyv vulnerabilities given the demonstrated
code execution capability?

I want to make sure I understand correctly before proceeding with the
advisory documentation.

@djc
Copy link
Copy Markdown
Member

djc commented Apr 24, 2026

Yup -- please update the advisory, then we can merge it. And I'd like to receive the technical details in private -- my email address should be easy to find, or you can find me on the rust-lang Zulip or Discord.

@tooson9010-spec
Copy link
Copy Markdown
Author

@djc Thank you for the approval! I've updated the advisory with the
arbitrary code execution findings as requested.

The advisory now includes:

  • Confirmed ACE vectors for both InlineVec::clear() and SerVec::clear()
  • Updated categories including "code-execution"
  • Technical impact assessment with CWE-94 classification

I'll send you the detailed exploitation research via email as discussed.

Ready for merge when you are!

Comment thread crates/rkyv/RUSTSEC-0000-0000.md Outdated
Comment thread crates/rkyv/RUSTSEC-0000-0000.md Outdated
Comment thread crates/rkyv/RUSTSEC-0000-0000.md Outdated
Comment thread crates/rkyv/RUSTSEC-0000-0000.md Outdated
Comment thread crates/rkyv/RUSTSEC-0000-0000.md Outdated
@tooson9010-spec
Copy link
Copy Markdown
Author

@bjorn3 Thank you for the detailed technical feedback. You're right about
the CWE-94 classification, URL fix, and keyword standardization.
I'll implement your suggested changes.

tooson9010-spec and others added 5 commits April 27, 2026 08:48
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants