Pengpeng Hou created THRIFT-5930:
------------------------------------
Summary: thrift_server_socket() copies Unix socket paths into
sockaddr_un.sun_path without bounds checking
Key: THRIFT-5930
URL: https://issues.apache.org/jira/browse/THRIFT-5930
Project: Thrift
Issue Type: Bug
Reporter: Pengpeng Hou
Hi,
I rechecked current upstream head of the C GLib transport and this still looks
like a real stack overflow bug.
In `lib/c_glib/src/thrift/c_glib/transport/thrift_server_socket.c`, the
Unix-socket bind path creates a stack `struct sockaddr_un pin`, zeroes it, sets
`sun_family`, and then copies `tsocket->path` with:
memcpy(pin.sun_path, tsocket->path, strlen(tsocket->path) + 1);
The same transport family has the matching client-side open path in
`thrift_socket.c`, but this issue is specifically about the server-side
bind/open path. The source path is stored via a GObject property and is not
length-validated before the copy. `sun_path` is a fixed-size field in `struct
sockaddr_un`, so an oversized Unix socket path can overflow the stack object
before bind/listen runs.
I rechecked the current code and did not find any local guard that limits the
copy to `sizeof(pin.sun_path) - 1`.
*How to reproduce*
1. Build current Apache Thrift with the C GLib transport.
2. Create or configure a `ThriftServerSocket` with a Unix domain socket path
longer than `sizeof(struct sockaddr_un.sun_path) - 1`.
3. Call the server-side open/bind path so it reaches the `memcpy()` into
`pin.sun_path`.
4. The copy overflows the local stack `sockaddr_un` object before the socket is
bound.
This is easiest to observe with ASan, but the bug is present even without it
because the destination is a fixed-size stack field and the copy length is
taken directly from `strlen(tsocket->path) + 1`.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)