pidgin/pidgin

Always ref result of g_io_stream_get_{input,output}_stream.

2021-03-05, Elliott Sales de Andrade
44c107f92a65
Parents 28d50eece92d
Children 3259645493ae
Always ref result of g_io_stream_get_{input,output}_stream.

Sometimes we wrap those in other streams, sometimes not. This makes prpls inconsistent with each other on whether they unref their `GInputStream`/`GOutputStream` in prpl data during shutdown. Thus, ref it when it's not wrapped, so we can _always_ do an unref in the cleanup.

This fixes a read-after-free Jabber shutdown, and presumably one in Novell, since it appears to do the same thing.

Testing Done:
Ran with valgrind and see no invalid reads in jabber shutdown. Novell was not tested, but seems similar.

Reviewed at https://reviews.imfreedom.org/r/558/
4 files changed, 16 insertions(+), 15 deletions(-)
  • +12 -11
    libpurple/protocols/bonjour/xmpp.c
  • +1 -1
    libpurple/protocols/jabber/jabber.c
  • +1 -1
    libpurple/protocols/novell/novell.c
  • +2 -2
    libpurple/protocols/sametime/sametime.c
    • --- a/libpurple/protocols/bonjour/xmpp.c Fri Mar 05 03:31:29 2021 -0600
    +++ b/libpurple/protocols/bonjour/xmpp.c Fri Mar 05 03:39:39 2021 -0600
    @@ -569,8 +569,8 @@
    G_INPUT_STREAM(bconv->input),
    G_OUTPUT_STREAM(bconv->output));
    g_clear_object(&bconv->socket);
    - bconv->input = NULL;
    - bconv->output = NULL;
    + g_clear_object(&bconv->input);
    + g_clear_object(&bconv->output);
    g_free(stream_start);
    return FALSE;
    @@ -633,8 +633,8 @@
    G_INPUT_STREAM(bconv->input),
    G_OUTPUT_STREAM(bconv->output));
    g_clear_object(&bconv->socket);
    - bconv->input = NULL;
    - bconv->output = NULL;
    + g_clear_object(&bconv->input);
    + g_clear_object(&bconv->output);
    /* This must be asynchronous because it destroys the parser and we
    * may be in the middle of parsing.
    @@ -716,9 +716,10 @@
    /* We wait for the stream start before doing anything else */
    bconv->socket = g_object_ref(connection);
    - bconv->input = g_io_stream_get_input_stream(G_IO_STREAM(bconv->socket));
    - bconv->output =
    - g_io_stream_get_output_stream(G_IO_STREAM(bconv->socket));
    + bconv->input = g_object_ref(
    + g_io_stream_get_input_stream(G_IO_STREAM(bconv->socket)));
    + bconv->output = g_object_ref(
    + g_io_stream_get_output_stream(G_IO_STREAM(bconv->socket)));
    source = g_pollable_input_stream_create_source(
    G_POLLABLE_INPUT_STREAM(bconv->input), bconv->cancellable);
    g_source_set_callback(source, (GSourceFunc)_client_socket_handler,
    @@ -845,9 +846,9 @@
    bb->conversation->socket = conn;
    bb->conversation->input =
    - g_io_stream_get_input_stream(G_IO_STREAM(conn));
    + g_object_ref(g_io_stream_get_input_stream(G_IO_STREAM(conn)));
    bb->conversation->output =
    - g_io_stream_get_output_stream(G_IO_STREAM(conn));
    + g_object_ref(g_io_stream_get_output_stream(G_IO_STREAM(conn)));
    if (!bonjour_xmpp_send_stream_init(bb->conversation, &error)) {
    PurpleConversation *conv = NULL;
    @@ -1168,8 +1169,8 @@
    /* Free all the data related to the conversation */
    g_clear_object(&bconv->socket);
    - bconv->input = NULL;
    - bconv->output = NULL;
    + g_clear_object(&bconv->input);
    + g_clear_object(&bconv->output);
    g_object_unref(G_OBJECT(bconv->tx_buf));
    if (bconv->stream_data != NULL) {
    --- a/libpurple/protocols/jabber/jabber.c Fri Mar 05 03:31:29 2021 -0600
    +++ b/libpurple/protocols/jabber/jabber.c Fri Mar 05 03:39:39 2021 -0600
    @@ -721,7 +721,7 @@
    GSource *source;
    js->stream = stream;
    - js->input = g_io_stream_get_input_stream(js->stream);
    + js->input = g_object_ref(g_io_stream_get_input_stream(js->stream));
    js->output = purple_queued_output_stream_new(
    g_io_stream_get_output_stream(js->stream));
    --- a/libpurple/protocols/novell/novell.c Fri Mar 05 03:31:29 2021 -0600
    +++ b/libpurple/protocols/novell/novell.c Fri Mar 05 03:39:39 2021 -0600
    @@ -1727,7 +1727,7 @@
    conn->stream = G_IO_STREAM(sockconn);
    conn->input =
    g_data_input_stream_new(g_io_stream_get_input_stream(conn->stream));
    - conn->output = g_io_stream_get_output_stream(conn->stream);
    + conn->output = g_object_ref(g_io_stream_get_output_stream(conn->stream));
    g_data_input_stream_set_byte_order(conn->input,
    G_DATA_STREAM_BYTE_ORDER_LITTLE_ENDIAN);
    --- a/libpurple/protocols/sametime/sametime.c Fri Mar 05 03:31:29 2021 -0600
    +++ b/libpurple/protocols/sametime/sametime.c Fri Mar 05 03:39:39 2021 -0600
    @@ -1749,7 +1749,7 @@
    }
    pd->stream = G_IO_STREAM(sockconn);
    - pd->input = g_io_stream_get_input_stream(pd->stream);
    + pd->input = g_object_ref(g_io_stream_get_input_stream(pd->stream));
    pd->output = purple_queued_output_stream_new(
    g_io_stream_get_output_stream(pd->stream));
    source = g_pollable_input_stream_create_source(
    @@ -3095,7 +3095,7 @@
    g_clear_object(&pd->cancellable);
    g_hash_table_destroy(pd->group_list_map);
    - pd->input = NULL;
    + g_clear_object(&pd->input);
    g_clear_object(&pd->output);
    g_clear_object(&pd->stream);