Skip to content

THRIFT-5930: c_glib: reject oversized Unix socket paths in thrift_server_socket_listen()#3354

Open
neosys007 wants to merge 1 commit intoapache:masterfrom
neosys007:codex/thrift-5930-server-sun_path
Open

THRIFT-5930: c_glib: reject oversized Unix socket paths in thrift_server_socket_listen()#3354
neosys007 wants to merge 1 commit intoapache:masterfrom
neosys007:codex/thrift-5930-server-sun_path

Conversation

@neosys007
Copy link
Contributor

This PR fixes the server-side Unix socket path copy in the C GLib transport.

In current head, thrift_server_socket_listen() builds a stack sockaddr_un and copies tsocket->path into sun_path with strlen(path) + 1. That only works when the configured path is shorter than the destination buffer; otherwise the copy can run past the end of the local sockaddr_un before bind() is even called.

The fix is intentionally small:

  • check the path length against sizeof(struct sockaddr_un.sun_path) first,
  • return a transport error if the path does not fit,
  • keep the rest of the server-side behavior unchanged.

I also added regression coverage in lib/c_glib/test/testtransportsocket.c that checks the server listen path rejects an overlong Unix socket path cleanly.

Validation performed locally:

  • git diff --check
  • syntax-only compile for the changed C files
  • the C GLib transport sources compile cleanly apart from existing OpenSSL deprecation warnings unrelated to this change.

Related Jira:

return nullptr;
}
strcpy(resolved_path, source);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strncpy()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right to call this out, but this file should not be part of THRIFT-5930 in the first place. This PR branch accidentally pulled in the separate saferealpath change from THRIFT-5932. I will clean up #3354 so it only contains the c_glib server-socket changes.

…ver_socket_listen()

thrift_server_socket_listen() still copies the configured Unix socket path directly into a stack sockaddr_un and then binds it. The path comes from a GObject property and is not checked against sizeof(pin.sun_path) before the copy.\n\nReject Unix socket paths that do not fit in the local sockaddr_un.sun_path buffer before building the sockaddr and binding the socket.
@neosys007 neosys007 force-pushed the codex/thrift-5930-server-sun_path branch from eba2263 to 69bb9b3 Compare March 25, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants