intel / intel/tsffs

Unsound Use of CStr::from_ptr in add_architecture_hint

Open
#123 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
331
Forks
26
Avg merge
1d 5h
Merged PRs (30d)
2

Description

**Description**
The add_architecture_hint function uses unsafe { CStr::from_ptr(hint) } to interpret a raw C string pointer (*mut c_char) as a CStr. This approach assumes that the pointer is valid, properly aligned, non-null, and null-terminated. If any of these conditions are violated, the program will invoke undefined behavior (UB). The function does not validate the hint pointer or its contents, making it unsound.
https://github.com/intel/tsffs/blob/1556d0facc804e35b5696622868ea7d0c3a4b989/src/interfaces/config.rs#L30
```
pub fn add_architecture_hint(&mut self, cpu: *mut ConfObject, hint: *mut c_char) -> Result<()> {
let hint = unsafe { CStr::from_ptr(hint) }.to_str()?;
let processor_number = get_processor_number(cpu)?;
debug!(
self.as_conf_object(),
"add_architecture_hint({processor_number}, {hint})"
);
self.architecture_hints
.insert(processor_number, ArchitectureHint::from_str(hint)?);

Ok(())
}
```
**Problems:**
this function is a` pub` function, so I assume user can control the `hint `field, it cause some problems.
1. Unchecked Pointer Validity:
The function does not verify that hint is a valid pointer. If hint is null, misaligned, or invalid, the call to CStr::from_ptr results in undefined behavior.
2. Null-Termination Requirement:
CStr::from_ptr requires the string to be null-terminated. If the input pointer does not point to a null-terminated string, the function will read out of bounds, causing undefined behavior.

The function does not document or enforce safety requirements for the hint parameter, leaving it up to the caller to ensure validity. This violates Rust's safety principles and makes the function unsound.

**Suggestion**
1. mark this function as `unsafe` and provide safety doc.
2. add some check in the function body.

**Additional Context:**
This issue arises from the unsafe handling of raw pointers and unchecked assumptions about input validity. Rust's unsafe constructs should only be used when their safety guarantees can be upheld, and all potential invalid states must be handled explicitly.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.