rustsec / rustsec/advisory-db

Unsoundness in yrs

Open
#3,118 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
No language data
Stars
1.2k
Forks
544
Avg merge
1d 10h
Merged PRs (30d)
45

Description

(Disclaimer: I am filing this disclosure personally and not on behalf of my employer)

YRS is a moderately popular (2M downloads according to crates.io) implementation of the y.js CRDT data structure, used for collaborative editing.

It is very unsound. Fundamental types are simple pointer wrappers and offer & and &mut access without any checks for aliasing or if the data has been deallocated, and fuzz testing reveals many panics and aborts from parsing untrusted input.

In particular, the BranchPtr structure, which forms the base of many other structures exposed in the public API, is a wrapper around a NonNull<Branch>. It implements Clone, Copy, Send, and Sync, but also offers a Deref and DerefMut that simply get a reference to the underlying object, leading to trivial aliasing issues. Additionally, it implements From<&Branch> and From<&Box<Branch>> and does no lifetime tracking, leading to trivial use-after-free issues.

As an example:

use yrs::{branch::Branch, Doc, Text as _, Transact};

fn main() {
    let doc = Doc::new();
    let item = doc.get_or_insert_text("test");

    let branch_ptr = AsRef::<Branch>::as_ref(&item).id().get_branch(
        &doc.transact()
    ).unwrap();
    std::mem::drop(doc); // item and branch_ptr are now now dangling

    branch_ptr.id(); // UB: branch_ptr is dangling

    let doc = Doc::new();
    let txn = doc.transact();
    item.len(&txn); // UB: access to dangling item
}

As another example, the Read::read_string trait in src/encoding/read.rs uses from_utf8_unchecked on untrusted input from a file.

Fuzzing the library (parsing and applying a document update to an empty doc) reveals a lot of panics and AddressSanitizer aborts. While I imagine some of the use-after-free issues could be leveraged into an actual exploit, I don't have the expertise needed to properly assess and document the issues as exploits.

I filed an issue about the BranchPtr unsoundness when I came across it a year ago as https://github.com/y-crdt/y-crdt/issues/506, and emailed the author with my findings from fuzzing. I never received a response to the email, and re-running the fuzzer today on the v0.27.3 tag still yields similar crashes.

Given that this is a library routinely exposed to untrusted input from many users, the existence of these issues and crashes is very concerning.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reported BranchPtr behavior and the earlier issue 506, then review src/encoding/read.rs and the fuzzing results for parsing and applying document updates. Determine whether the reported unsoundness and crashes are sufficiently verified and scoped for an advisory, with v0.27.3 and affected behavior documented as evidence.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.