Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible soundness bug: alignment not checked #50

Open
Plecra opened this issue Jul 4, 2021 · 1 comment · May be fixed by #51
Open

Possible soundness bug: alignment not checked #50

Plecra opened this issue Jul 4, 2021 · 1 comment · May be fixed by #51

Comments

@Plecra
Copy link

Plecra commented Jul 4, 2021

atty/src/lib.rs

Lines 131 to 141 in 7b5df17

let mut name_info_bytes = vec![0u8; size + MAX_PATH * mem::size_of::<WCHAR>()];
let res = GetFileInformationByHandleEx(
GetStdHandle(fd),
FileNameInfo,
&mut *name_info_bytes as *mut _ as *mut c_void,
name_info_bytes.len() as u32,
);
if res == 0 {
return false;
}
let name_info: &FILE_NAME_INFO = &*(name_info_bytes.as_ptr() as *const FILE_NAME_INFO);

As far as I can tell, the pointer deference on line 141 in unsound, as there is no guarantee the vector will be properly aligned for FILE_NAME_INFO (which has an alignment of 4 due to FileNameLength being a u32)

@Plecra Plecra linked a pull request Jul 4, 2021 that will close this issue
Shnatsel added a commit to rust-secure-code/cargo-supply-chain that referenced this issue Nov 4, 2022
Techcable added a commit to slog-rs/term that referenced this issue Nov 28, 2022
Switches to newer `is-terminal` crate instead.
This functionality is also availible on the nightly
Rust stdlib as a `std::io::IsTerminal` trait.

Avoids RUSTSEC-2021-0145 (softprops/atty#50)
Fixes slog-rs/slog#319

Based on the information in the vulnerability database,
I don't consider this a particularly serious bug.

> In practice however, the pointer won't be unaligned
   unless a custom global allocator is used.
@softprops
Copy link
Owner

now recommending users move to the std.io.IsTerminal available since rust 1.17.0 in this repos readme

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants