Skip to content

xcb connect_to_fd* constructors unsound #2355

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

En-En-Code
Copy link

Identifies a flaw in xcb::Connection::connect_to_fd and xcb::Connection::connect_to_fd_with_extensions which allows file descriptors to be used after close and/or double closed. An issue was raised back in 2022 to mark these functions as unsafe with no additional context. I discovered this unsoundness causing problems working on a different project and reported my findings to xcb.
It is unclear as to whether the project is maintained, though previously there was an issue raised as to its maintained status (see #653).
I did not put any categories or keywords. I just did not know what to put for them.

@djc
Copy link
Contributor

djc commented Aug 13, 2025

I've emailed @rtbo personally (they seem somewhat active on GitHub at least), let's see if they respond in the next two weeks. If not, we should go ahead and merge the advisory (please help remind me).

@rtbo
Copy link

rtbo commented Aug 14, 2025

Hi,
I've honestly little time to allocate to the maintenance of rust-xcb, but I don't want it to die unmaintained either!
I can definitely share a write access to the repo.

@fintelia
Copy link

One thing to mention is that this isn't a memory safety violation but rather an I/O safety violation. I think that may warrant tagging it with a dedicated category? It also wasn't clear to me from reading the discussions linked from that RFC whether I/O safety violations can cause undefined behavior, and thus whether this qualifies as "unsound" at all.

I also experimented with the nix crate a bit and discovered that you can trigger the same crash described here simply by opening a file with File::open, getting its raw file descriptor via as_raw_fd, and then closing it with both the (safe) nix::unistd::close file and via dropping the File. If this really constitutes undefined behavior... I feel like a crate as widely used as nix would've been fixed already?

@rtbo
Copy link

rtbo commented Aug 16, 2025

Should be closed by rust-x-bindings/rust-xcb#283

@djc
Copy link
Contributor

djc commented Aug 16, 2025

One thing to mention is that this isn't a memory safety violation but rather an I/O safety violation. I think that may warrant tagging it with a dedicated category? It also wasn't clear to me from reading the discussions linked from that RFC whether I/O safety violations can cause undefined behavior, and thus whether this qualifies as "unsound" at all.

That's a good point, this probably doesn't qualify as unsound.

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

Successfully merging this pull request may close these issues.

4 participants