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

SetFileInformationByHandle and friends are difficult to use soundly #2044

Closed
chyyran opened this issue Sep 21, 2022 · 2 comments · Fixed by #2046
Closed

SetFileInformationByHandle and friends are difficult to use soundly #2044

chyyran opened this issue Sep 21, 2022 · 2 comments · Fixed by #2046
Labels
bug Something isn't working

Comments

@chyyran
Copy link

chyyran commented Sep 21, 2022

In 0.39.0, APIs that vary based their input or output based on FILE_INFO_BY_HANDLE_CLASS were usable like so with good confidence in their soundness.

let basic_info = FILE_BASIC_INFO {
      FileAttributes: if file_attributes == FILE_FLAGS_AND_ATTRIBUTES(0) {
          FILE_ATTRIBUTE_NORMAL
      } else {
          file_attributes
      }
      .0,
      ..Default::default()
};
SetFileInformationByHandle(
    handle,
    FileBasicInfo,
    (&basic_info as *const FILE_BASIC_INFO).cast(),
    std::mem::size_of::<FILE_BASIC_INFO>() as u32,
)

With the changes in 0.40.0, SetFileInformationByHandle now takes &[u8] instead of a pointer and a size. There is no API guarantee that &FILE_BASIC_INFO is punnable to &[u8] without a blessed way of converting between the two and is highly likely to be UB. transmute is definitely not sound here. MaybeUninit is a non-starter because creating a &[u8] reference by casting the pointer into *const u8 and taking a reference is immediate UB.

Friends such as GetFileInformationByHandleEx are also difficult to use in the same way, and may be even worse from the UB side because a mutable reference is required.

@chyyran chyyran changed the title SetFileInformationByHandle and friends are difficult to use soundly. SetFileInformationByHandle and friends are difficult to use soundly Sep 21, 2022
@chyyran
Copy link
Author

chyyran commented Sep 21, 2022

Possibly a dupe of #2034?

@kennykerr kennykerr added the bug Something isn't working label Sep 21, 2022
@kennykerr
Copy link
Collaborator

Thanks for reporting! Yes, #2034 is related. #2037 partially fixed this issue by avoiding the array transformation when the metadata wasn't matching the semantics expected of certain types. But in fixing that, I included *c_void types along with *u8 and that hits into APIs like this that end up implying polymorphism. So I've fixed that in an upcoming PR to further exclude APIs with such types from the array transformation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants