Permalink
Browse files

purple: fix file transfer memory management

This means cancelling transfers on logout to avoid crashes, keeping
track of timeouts, reffing and unreffing the xfers, listening to the
callbacks from UI and purple more carefully and using the correct
functions to free the correct things at the correct moments.

Originally intended to fix a crash triggered when the dcc stall timeout
kicks in after the account is offline, which is apparently very frequent
with telegram (it sends file transfers while fetching history, and
randomly disconnects a while later).

Trying to fix that meant opening a can of worms, but after three days of
work on this bug I'm pretty sure I've finished dealing with the
resulting mess and tested all the typical edge cases.
  • Loading branch information...
dequis committed Nov 13, 2016
1 parent 701ab81 commit ea902752503fc5b356d6513911081ec932d804f2
Showing with 75 additions and 11 deletions.
  1. +1 −0 protocols/purple/bpurple.h
  2. +68 −11 protocols/purple/ft.c
  3. +6 −0 protocols/purple/purple.c
@@ -13,6 +13,7 @@ struct purple_data
GHashTable *input_requests;
guint next_request_id;
char *chat_list_server;
GSList *filetransfers;
};

#endif /* !BPURPLE_H */
@@ -41,6 +41,7 @@ struct prpl_xfer_data {
int fd;
char *fn, *handle;
gboolean ui_wants_data;
int timeout;
};

static file_transfer_t *next_ft;
@@ -63,20 +64,57 @@ static void prpl_xfer_canceled(struct file_transfer *ft, char *reason)
{
struct prpl_xfer_data *px = ft->data;

purple_xfer_request_denied(px->xfer);
if (px->xfer) {
if (!purple_xfer_is_completed(px->xfer) && !purple_xfer_is_canceled(px->xfer)) {
purple_xfer_cancel_local(px->xfer);
}
px->xfer->ui_data = NULL;
purple_xfer_unref(px->xfer);
px->xfer = NULL;
}
}

static void prpl_xfer_free(struct file_transfer *ft)
{
struct prpl_xfer_data *px = ft->data;
struct purple_data *pd = px->ic->proto_data;

pd->filetransfers = g_slist_remove(pd->filetransfers, px);

if (px->xfer) {
px->xfer->ui_data = NULL;
purple_xfer_unref(px->xfer);
}

if (px->timeout) {
b_event_remove(px->timeout);
}

g_free(px->fn);
g_free(px->handle);
if (px->fd >= 0) {
close(px->fd);
}
g_free(px);
}

static void prplcb_xfer_new(PurpleXfer *xfer)
{
purple_xfer_ref(xfer);

if (purple_xfer_get_type(xfer) == PURPLE_XFER_RECEIVE) {
struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1);
struct purple_data *pd;

xfer->ui_data = px;
px->xfer = xfer;
px->fn = mktemp(g_strdup("/tmp/bitlbee-purple-ft.XXXXXX"));
px->fd = -1;
px->ic = purple_ic_by_pa(xfer->account);

pd = px->ic->proto_data;
pd->filetransfers = g_slist_prepend(pd->filetransfers, px);

purple_xfer_set_local_filename(xfer, px->fn);

/* Sadly the xfer struct is still empty ATM so come back after
@@ -111,6 +149,7 @@ static gboolean prplcb_xfer_new_send_cb(gpointer data, gint fd, b_input_conditio

px->ft->accept = prpl_xfer_accept;
px->ft->canceled = prpl_xfer_canceled;
px->ft->free = prpl_xfer_free;
px->ft->write_request = prpl_xfer_write_request;

return FALSE;
@@ -163,17 +202,13 @@ static gboolean prpl_xfer_write_request(struct file_transfer *ft)
}


/* Generic (IM<>UI): */
static void prplcb_xfer_destroy(PurpleXfer *xfer)
{
struct prpl_xfer_data *px = xfer->ui_data;

g_free(px->fn);
g_free(px->handle);
if (px->fd >= 0) {
close(px->fd);
if (px) {
px->xfer = NULL;
}
g_free(px);
}

static void prplcb_xfer_progress(PurpleXfer *xfer, double percent)
@@ -223,9 +258,9 @@ static void prplcb_xfer_cancel_remote(PurpleXfer *xfer)
{
struct prpl_xfer_data *px = xfer->ui_data;

if (px->ft) {
if (px && px->ft) {
imcb_file_canceled(px->ic, px->ft, "Canceled by remote end");
} else {
} else if (px) {
/* px->ft == NULL for sends, because of the two stages. :-/ */
imcb_error(px->ic, "File transfer cancelled by remote end");
}
@@ -239,10 +274,12 @@ static gboolean purple_transfer_request_cb(gpointer data, gint fd, b_input_condi
void purple_transfer_request(struct im_connection *ic, file_transfer_t *ft, char *handle)
{
struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1);
struct purple_data *pd;
char *dir, *basename;

ft->data = px;
px->ft = ft;
px->ft->free = prpl_xfer_free;

dir = g_strdup("/tmp/bitlbee-purple-ft.XXXXXX");
if (!mkdtemp(dir)) {
@@ -271,10 +308,13 @@ void purple_transfer_request(struct im_connection *ic, file_transfer_t *ft, char
px->ic = ic;
px->handle = g_strdup(handle);

pd = px->ic->proto_data;
pd->filetransfers = g_slist_prepend(pd->filetransfers, px);

imcb_log(ic,
"Due to libpurple limitations, the file has to be cached locally before proceeding with the actual file transfer. Please wait...");

b_timeout_add(0, purple_transfer_request_cb, ft);
px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft);
}

static void purple_transfer_forward(struct file_transfer *ft)
@@ -294,6 +334,8 @@ static gboolean purple_transfer_request_cb(gpointer data, gint fd, b_input_condi
file_transfer_t *ft = data;
struct prpl_xfer_data *px = ft->data;

px->timeout = 0;

if (ft->write == NULL) {
ft->write = prpl_xfer_write;
imcb_file_recv_start(px->ic, ft);
@@ -321,12 +363,27 @@ static gboolean prpl_xfer_write(struct file_transfer *ft, char *buffer, unsigned
imcb_file_finished(px->ic, ft);
px->ft = NULL;
} else {
b_timeout_add(0, purple_transfer_request_cb, ft);
px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft);
}

return TRUE;
}

void purple_transfer_cancel_all(struct im_connection *ic)
{
struct purple_data *pd = ic->proto_data;

while (pd->filetransfers) {
struct prpl_xfer_data *px = pd->filetransfers->data;

if (px->ft) {
imcb_file_canceled(ic, px->ft, "Logging out");
}

pd->filetransfers = g_slist_remove(pd->filetransfers, px);
}
}



PurpleXferUiOps bee_xfer_uiops =
@@ -42,6 +42,8 @@ static char *set_eval_display_name(set_t *set, char *value);
void purple_request_input_callback(guint id, struct im_connection *ic,
const char *message, const char *who);

void purple_transfer_cancel_all(struct im_connection *ic);

/* purple_request_input specific stuff */
typedef void (*ri_callback_t)(gpointer, const gchar *);

@@ -384,6 +386,10 @@ static void purple_logout(struct im_connection *ic)
imcb_chat_free(ic->groupchats->data);
}

if (pd->filetransfers) {
purple_transfer_cancel_all(ic);
}

purple_account_set_enabled(pd->account, "BitlBee", FALSE);
purple_connections = g_slist_remove(purple_connections, ic);
purple_accounts_remove(pd->account);

2 comments on commit ea90275

@carnil

This comment has been minimized.

@dequis

This comment has been minimized.

Copy link
Member Author

dequis replied Jan 31, 2017

CVE-2016-10188 has been assigned for this.

Please sign in to comment.