Skip to content

Commit 69bb9b3

Browse files
committed
THRIFT-5930: c_glib: reject oversized Unix socket paths in thrift_server_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.
1 parent 3b0ab4d commit 69bb9b3

2 files changed

Lines changed: 65 additions & 2 deletions

File tree

lib/c_glib/src/thrift/c_glib/transport/thrift_server_socket.c

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@ enum _ThriftServerSocketProperties
4949

5050
G_DEFINE_TYPE(ThriftServerSocket, thrift_server_socket, THRIFT_TYPE_SERVER_TRANSPORT)
5151

52+
static gboolean
53+
thrift_server_socket_path_fits_unix_addr (const gchar *path, GError **error)
54+
{
55+
const size_t path_len = strlen (path);
56+
const size_t sun_path_len = sizeof (((struct sockaddr_un *) 0)->sun_path);
57+
58+
if (path_len >= sun_path_len)
59+
{
60+
if (error != NULL)
61+
{
62+
g_set_error (error,
63+
THRIFT_SERVER_SOCKET_ERROR,
64+
THRIFT_SERVER_SOCKET_ERROR_BIND,
65+
"unix socket path is too long (%zu bytes, max %zu)",
66+
path_len, sun_path_len - 1);
67+
}
68+
return FALSE;
69+
}
70+
71+
return TRUE;
72+
}
73+
5274
gboolean
5375
thrift_server_socket_listen (ThriftServerTransport *transport, GError **error)
5476
{
@@ -80,6 +102,13 @@ thrift_server_socket_listen (ThriftServerTransport *transport, GError **error)
80102
/* bind to the socket */
81103
if (tsocket->path)
82104
{
105+
if (!thrift_server_socket_path_fits_unix_addr (tsocket->path, error))
106+
{
107+
close (tsocket->sd);
108+
tsocket->sd = THRIFT_INVALID_SOCKET;
109+
return FALSE;
110+
}
111+
83112
/* create a socket structure */
84113
struct sockaddr_un pin;
85114
memset (&pin, 0, sizeof(pin));
@@ -374,4 +403,3 @@ thrift_server_socket_class_init (ThriftServerSocketClass *cls)
374403
tstc->accept = thrift_server_socket_accept;
375404
tstc->close = thrift_server_socket_close;
376405
}
377-

lib/c_glib/test/testtransportsocket.c

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
#include <netdb.h>
21+
#include <sys/un.h>
2122
#include <sys/wait.h>
2223

2324
#include <thrift/c_glib/transport/thrift_transport.h>
@@ -71,6 +72,18 @@ my_send(int socket, const void *buffer, size_t length, int flags)
7172

7273
static void thrift_socket_server (const int port);
7374
static void thrift_socket_server_open (const int port, int times);
75+
76+
static gchar *
77+
make_too_long_unix_socket_path (void)
78+
{
79+
struct sockaddr_un addr;
80+
const size_t path_len = sizeof (addr.sun_path) + 1;
81+
gchar *path = g_malloc (path_len + 1);
82+
83+
memset (path, 'a', path_len);
84+
path[path_len] = '\0';
85+
return path;
86+
}
7487
/* test object creation and destruction */
7588
static void
7689
test_create_and_destroy(void)
@@ -288,6 +301,28 @@ test_peek(void)
288301
}
289302
}
290303

304+
static void
305+
test_server_listen_rejects_too_long_unix_path (void)
306+
{
307+
ThriftServerSocket *tsocket = NULL;
308+
ThriftServerTransport *transport = NULL;
309+
GError *error = NULL;
310+
gchar *path = make_too_long_unix_socket_path ();
311+
312+
tsocket = g_object_new (THRIFT_TYPE_SERVER_SOCKET,
313+
"path", path,
314+
NULL);
315+
transport = THRIFT_SERVER_TRANSPORT (tsocket);
316+
317+
g_assert (thrift_server_transport_listen (transport, &error) == FALSE);
318+
g_assert_error (error, THRIFT_SERVER_SOCKET_ERROR, THRIFT_SERVER_SOCKET_ERROR_BIND);
319+
g_clear_error (&error);
320+
g_assert (tsocket->sd == THRIFT_INVALID_SOCKET);
321+
322+
g_object_unref (tsocket);
323+
g_free (path);
324+
}
325+
291326
static void
292327
thrift_socket_server_open (const int port, int times)
293328
{
@@ -355,7 +390,7 @@ main(int argc, char *argv[])
355390
g_test_add_func ("/testtransportsocket/OpenAndClose", test_open_and_close);
356391
g_test_add_func ("/testtransportsocket/ReadAndWrite", test_read_and_write);
357392
g_test_add_func ("/testtransportsocket/Peek", test_peek);
393+
g_test_add_func ("/testtransportsocket/ServerListenRejectsTooLongUnixPath", test_server_listen_rejects_too_long_unix_path);
358394

359395
return g_test_run ();
360396
}
361-

0 commit comments

Comments
 (0)