Skip to content

cell::Ref::clone not hardened against "optimized mem::forget in a loop" attacks. #33880

Closed
@eddyb

Description

@eddyb

Check out in Release mode on playpen:

#![feature(cell_extras)]
use std::cell::{RefCell, Ref};
use std::mem;
use std::usize;

fn main() {
    let ref_cell = RefCell::new(vec![1, 2, 3]);
    let r = ref_cell.borrow();
    let mut i = 0;
    while i < usize::MAX {
        mem::forget(Ref::clone(&r));
        i += 1;
    }
    ref_cell.borrow_mut().push(r[0]);
}

This succeeds (when compiled with optimizations) despite having both a Ref and a RefMut.
RefCell::borrow is safe from this because at usize::MAX it believes it's mutably borrowed and panics.

There's a debug_assert!(borrow != WRITING && borrow != UNUSED); line in src/libcore/cell.rs which would catch this with debug assertions enabled, I believe it should be an assert! (Ref::clone is unstable and likely not performance critical anyway).

cc @ubsan @alexcrichton

Metadata

Metadata

Assignees

No one assigned

    Labels

    E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions