Pengpeng Hou created THRIFT-5933:
------------------------------------
Summary: thrift_socket_open() copies Unix socket paths into
sockaddr_un.sun_path without length validation
Key: THRIFT-5933
URL: https://issues.apache.org/jira/browse/THRIFT-5933
Project: Thrift
Issue Type: Bug
Reporter: Pengpeng Hou
Hello,
I reviewed current upstream head of the C GLib transport and found a real stack
overflow bug in the Unix-socket open path.
In `lib/c_glib/src/thrift/c_glib/transport/thrift_socket.c`, the Unix-socket
open 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 path is populated via the GObject property setter and is simply duplicated
with `g_strdup(g_value_get_string(value))`. There is no length validation at
that point, and there is no local guard before the copy into `sun_path`.
Because `sun_path` is a fixed-size field in `struct sockaddr_un`, an oversized
Unix socket path can overflow the stack object before `connect()` is even
called.
This is the client-side twin of the same Unix-socket-path problem in the server
transport. If you prefer to keep the reports separate, this one can stay
focused on `thrift_socket_open()` only.
*How to reproduce*
1. Build current Apache Thrift with the C GLib transport.
2. Configure a `ThriftSocket` with a Unix domain socket path longer than
`sizeof(struct sockaddr_un.sun_path) - 1`.
3. Call `thrift_socket_open()`.
4. The function copies the entire string into `pin.sun_path` with `memcpy()`
and overflows the local stack `sockaddr_un`.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
