Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

UB in SockaddrStorage::hash #1939

Closed
asomers opened this issue Dec 10, 2022 · 1 comment
Closed

UB in SockaddrStorage::hash #1939

asomers opened this issue Dec 10, 2022 · 1 comment
Labels

Comments

@asomers
Copy link
Member

asomers commented Dec 10, 2022

SockaddrStorage::from_raw allows creating a variable that isn't fully initialized. But the SockaddrStorage::hash implementation assumes that if the ss_family field is of an unknown type, then the entire structure must be fully initialized. You could cause it to access uninitialized data by doing something like the following:

let sa = libc::sockaddr {
    sa_len: 8,
    sa_family: 255,  // Not a valid family
    .. unsafe{ mem::zeroed() }
};
let ss = unsafe{ SockaddrStorage::from_raw((&sa).as_ptr(), Some(sa.sa_len)).unwrap() };
let mut s = DefaultHasher::new();
ss.hash(&mut s);

Granted, this requires the use of unsafe. But we can and should still fix it by always validating sa_family in SockaddrStorage::from_raw.

@asomers asomers added the A-bug label Dec 10, 2022
@Jan561 Jan561 mentioned this issue Nov 23, 2023
3 tasks
@asomers
Copy link
Member Author

asomers commented Nov 23, 2023

Actually, this isn't a bug. When I wrote it up, I assumed that the sockaddr_storage would be not fully initialized. But actually, that was never true. It's always been zero-initialized in this case. See

let mut ss: libc::sockaddr_storage = unsafe { mem::zeroed() };

@asomers asomers closed this as completed Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant