Skip to content

close _ignores_ EBADF (.badFileDescriptor) which is super dangerous, needs to preconditionFailure this #37

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

Closed
weissi opened this issue May 1, 2025 · 1 comment · Fixed by #38
Assignees
Labels
blocker Show stopping issues for 0.0.1 bug Something isn't working

Comments

@weissi
Copy link

weissi commented May 1, 2025

if errno != .badFileDescriptor {
throw errno
}

This is an extremely dangerous line of code. If you every hit EBADF on close then you know all bets are off and the only thing you should do is crash. Why so extreme? If some piece of code lost track of the file descriptors and closed 'the wrong one', security vulnerabilities will happen.

For example:

A confused piece of code accidentally double-closes

let fd = openSomething() // A
defer {
    try? close(fd) // B
}

[...]

write(fd, "some data", ...) // C

try? close(fd) // D

this looks harmless because it looks somewhat sensible that close would be able to detect a failure to close. But that is not true on UNIX because POSIX guarantees to always use the lowest available fd number.

So the above piece of code will absolutely close an unrelated file descriptor if some other thread opens a file descriptor between point A and D which is very likely. Now it might sound kinda harmless to accidentally close an unrelated file descriptor but it's really not. This is how security vulnerabilities are made: If we closed an unrelated file descriptor a 'hole' opens up in the file descriptor table that will often promptly be filled with something else. This means that writes of sensitive data might hit unrelated network connections or files. I have personally seen a process that I worked on that has sent sensitive data means for one HTTP connection into another HTTP connection by accident. It was medical data...

@weissi
Copy link
Author

weissi commented May 1, 2025

The only correct way to handle EBADF is to preconditionFailure("hit EBADF -- something closed owned file descriptor \(fd)"). See SwiftNIO code which handles that correctly.

@iCharlesHu iCharlesHu self-assigned this May 9, 2025
@iCharlesHu iCharlesHu added bug Something isn't working blocker Show stopping issues for 0.0.1 labels May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Show stopping issues for 0.0.1 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants