Maybe unsound in RLocal::new
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 337
- Forks
- 32
- Avg merge
- 8d 17h
- Merged PRs (30d)
- 11
Description
Hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project.
I notice the following code:
pub struct RLocal<T: Copy> {
old_value: T,
variable: *mut T,
}
impl<T> RLocal<T>
where
T: Copy,
{
pub fn new(new_value: T, variable: *mut T) -> RLocal<T> {
unsafe {
let old_value = libr::get(variable);
libr::set(variable, new_value);
Self {
old_value,
variable,
}
}
}
}
Considering that pub mod raii, and new is also a pub function. I assume that users can directly call this function. This potential situation could result in libr::get being called to a null pointer, and might trigger undefined behavior (UB). For safety reasons, I felt it necessary to report this issue. If you have performed checks elsewhere that ensure this is safe, please don’t take offense at my raising this issue.
I suggest Several possible fixes:
- If there is no external usage for
RLocalornew, they should not marked aspub, at least itsnewshould not marked aspub newmethod should add additional check for null pointer.- mark new method as unsafe and proper doc to let users know that they should provide valid Pointers.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the public raii module and RLocal::new, then inspect callers and the pointer assumptions around libr::get and libr::set. Determine whether the API can receive a null or otherwise invalid pointer; done means the safety contract or visibility is unambiguous and the reported undefined-behavior path is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100