From 0eb5722af68df00bc21806752e32ec0ab67a3eb9 Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Thu, 24 Mar 2016 18:26:46 -0400 Subject: [PATCH] rfbsrc: Check for read/write error Check for read/write error. This prevent undefined behaviour that rely on unitialized buffer. --- gst/librfb/rfbdecoder.c | 175 ++++++++++++++++++++++++++++------------ 1 file changed, 125 insertions(+), 50 deletions(-) diff --git a/gst/librfb/rfbdecoder.c b/gst/librfb/rfbdecoder.c index 038f6046ad..9142f1dcbe 100644 --- a/gst/librfb/rfbdecoder.c +++ b/gst/librfb/rfbdecoder.c @@ -35,16 +35,16 @@ static gboolean rfb_decoder_state_framebuffer_update_rectangle (RfbDecoder * decoder); static gboolean rfb_decoder_state_set_colour_map_entries (RfbDecoder * decoder); static gboolean rfb_decoder_state_server_cut_text (RfbDecoder * decoder); -static void rfb_decoder_raw_encoding (RfbDecoder * decoder, gint start_x, +static gboolean rfb_decoder_raw_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h); -static void rfb_decoder_copyrect_encoding (RfbDecoder * decoder, gint start_x, +static gboolean rfb_decoder_copyrect_encoding (RfbDecoder * decoder, + gint start_x, gint start_y, gint rect_w, gint rect_h); +static gboolean rfb_decoder_rre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h); -static void rfb_decoder_rre_encoding (RfbDecoder * decoder, gint start_x, - gint start_y, gint rect_w, gint rect_h); -static void rfb_decoder_corre_encoding (RfbDecoder * decoder, gint start_x, - gint start_y, gint rect_w, gint rect_h); -static void rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, +static gboolean rfb_decoder_corre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h); +static gboolean rfb_decoder_hextile_encoding (RfbDecoder * decoder, + gint start_x, gint start_y, gint rect_w, gint rect_h); RfbDecoder * rfb_decoder_new (void) @@ -189,7 +189,9 @@ rfb_decoder_read (RfbDecoder * decoder, guint32 len) GInputStream *in; GError *err = NULL; - g_return_val_if_fail (decoder->connection != NULL, NULL); + if (!decoder->connection) + return FALSE; + g_return_val_if_fail (len > 0, NULL); in = g_io_stream_get_input_stream (G_IO_STREAM (decoder->connection)); @@ -230,7 +232,9 @@ rfb_decoder_send (RfbDecoder * decoder, guint8 * buffer, guint len) GOutputStream *out; GError *err = NULL; - g_return_val_if_fail (decoder->connection != NULL, 0); + if (!decoder->connection) + return FALSE; + g_return_val_if_fail (buffer != NULL, 0); g_return_val_if_fail (len > 0, 0); @@ -333,7 +337,8 @@ rfb_decoder_state_wait_for_protocol_version (RfbDecoder * decoder) { gchar version_str[] = "RFB 003.003\n"; - rfb_decoder_read (decoder, 12); + if (!rfb_decoder_read (decoder, 12)) + return FALSE; g_return_val_if_fail (memcmp (decoder->data, "RFB 003.00", 10) == 0, FALSE); g_return_val_if_fail (*(decoder->data + 11) == 0x0a, FALSE); @@ -365,7 +370,9 @@ rfb_decoder_state_wait_for_protocol_version (RfbDecoder * decoder) } version_str[10] = '0' + decoder->protocol_minor; - rfb_decoder_send (decoder, (guint8 *) version_str, 12); + + if (!rfb_decoder_send (decoder, (guint8 *) version_str, 12)) + return FALSE; decoder->state = rfb_decoder_state_wait_for_security; return TRUE; @@ -380,10 +387,14 @@ rfb_decoder_state_reason (RfbDecoder * decoder) { gint reason_length; - rfb_decoder_read (decoder, 4); + if (!rfb_decoder_read (decoder, 4)) + return FALSE; reason_length = RFB_GET_UINT32 (decoder->data); - rfb_decoder_read (decoder, reason_length); + + if (!rfb_decoder_read (decoder, reason_length)) + return FALSE; + GST_WARNING ("Reason by server: %s", decoder->data); if (decoder->error == NULL) { @@ -405,7 +416,8 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) * above. */ if (IS_VERSION_3_3 (decoder)) { - rfb_decoder_read (decoder, 4); + if (!rfb_decoder_read (decoder, 4)) + return FALSE; decoder->security_type = RFB_GET_UINT32 (decoder->data); GST_DEBUG ("security = %d", decoder->security_type); @@ -421,7 +433,8 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) gint i; guint8 *type = NULL; - rfb_decoder_read (decoder, 1); + if (!rfb_decoder_read (decoder, 1)) + return FALSE; num_type = RFB_GET_UINT8 (decoder->data); if (num_type == 0) { @@ -429,7 +442,9 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) return TRUE; } - rfb_decoder_read (decoder, num_type); + if (!rfb_decoder_read (decoder, num_type)) + return FALSE; + decoder->security_type = SECURITY_FAIL; /* For now, simply pick the first support security method */ @@ -453,7 +468,8 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) } GST_DEBUG ("security = %d", decoder->security_type); - rfb_decoder_send (decoder, type, 1); + if (!rfb_decoder_send (decoder, type, 1)) + return FALSE; } switch (decoder->security_type) { @@ -502,7 +518,8 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) des (&des_ctx, challenge + 8, challenge + 8); /* .. and send back to server */ - rfb_decoder_send (decoder, challenge, 16); + if (!rfb_decoder_send (decoder, challenge, 16)) + return FALSE; GST_DEBUG ("Encrypted challenge sent to server"); @@ -524,7 +541,9 @@ rfb_decoder_state_wait_for_security (RfbDecoder * decoder) static gboolean rfb_decoder_state_security_result (RfbDecoder * decoder) { - rfb_decoder_read (decoder, 4); + if (!rfb_decoder_read (decoder, 4)) + return FALSE; + if (RFB_GET_UINT32 (decoder->data) != 0) { GST_WARNING ("Security handshaking failed"); if (IS_VERSION_3_8 (decoder)) { @@ -598,7 +617,11 @@ rfb_decoder_state_set_encodings (RfbDecoder * decoder) message = rfb_decoder_message_set_encodings (encoder_list); - rfb_decoder_send (decoder, message, 4 + 4 * g_slist_length (encoder_list)); + if (!rfb_decoder_send (decoder, message, + 4 + 4 * g_slist_length (encoder_list))) { + g_free (message); + return FALSE; + } g_free (message); @@ -614,7 +637,10 @@ rfb_decoder_state_send_client_initialisation (RfbDecoder * decoder) guint8 shared_flag; shared_flag = decoder->shared_flag; - rfb_decoder_send (decoder, &shared_flag, 1); + + if (!rfb_decoder_send (decoder, &shared_flag, 1)) + return FALSE; + GST_DEBUG ("shared_flag is %d", shared_flag); decoder->state = rfb_decoder_state_wait_for_server_initialisation; @@ -626,7 +652,8 @@ rfb_decoder_state_wait_for_server_initialisation (RfbDecoder * decoder) { guint32 name_length; - rfb_decoder_read (decoder, 24); + if (!rfb_decoder_read (decoder, 24)) + return FALSE; decoder->width = RFB_GET_UINT16 (decoder->data + 0); decoder->height = RFB_GET_UINT16 (decoder->data + 2); @@ -657,7 +684,8 @@ rfb_decoder_state_wait_for_server_initialisation (RfbDecoder * decoder) name_length = RFB_GET_UINT32 (decoder->data + 20); - rfb_decoder_read (decoder, name_length); + if (!rfb_decoder_read (decoder, name_length)) + return FALSE; decoder->name = g_strndup ((gchar *) (decoder->data), name_length); GST_DEBUG ("name = %s", decoder->name); @@ -714,7 +742,9 @@ rfb_decoder_state_normal (RfbDecoder * decoder) GST_DEBUG ("decoder_state_normal"); - rfb_decoder_read (decoder, 1); + if (!rfb_decoder_read (decoder, 1)) + return FALSE; + message_type = RFB_GET_UINT8 (decoder->data); switch (message_type) { @@ -743,7 +773,8 @@ static gboolean rfb_decoder_state_framebuffer_update (RfbDecoder * decoder) { - rfb_decoder_read (decoder, 3); + if (!rfb_decoder_read (decoder, 3)) + return FALSE; decoder->n_rects = RFB_GET_UINT16 (decoder->data + 1); GST_DEBUG ("Number of rectangles : %d", decoder->n_rects); @@ -758,8 +789,10 @@ rfb_decoder_state_framebuffer_update_rectangle (RfbDecoder * decoder) { gint x, y, w, h; gint encoding; + gboolean ret = FALSE; - rfb_decoder_read (decoder, 12); + if (!rfb_decoder_read (decoder, 12)) + return FALSE; x = RFB_GET_UINT16 (decoder->data + 0) - decoder->offset_x; y = RFB_GET_UINT16 (decoder->data + 2) - decoder->offset_y; @@ -780,34 +813,39 @@ rfb_decoder_state_framebuffer_update_rectangle (RfbDecoder * decoder) switch (encoding) { case ENCODING_TYPE_RAW: - rfb_decoder_raw_encoding (decoder, x, y, w, h); + ret = rfb_decoder_raw_encoding (decoder, x, y, w, h); break; case ENCODING_TYPE_COPYRECT: - rfb_decoder_copyrect_encoding (decoder, x, y, w, h); + ret = rfb_decoder_copyrect_encoding (decoder, x, y, w, h); break; case ENCODING_TYPE_RRE: - rfb_decoder_rre_encoding (decoder, x, y, w, h); + ret = rfb_decoder_rre_encoding (decoder, x, y, w, h); break; case ENCODING_TYPE_CORRE: - rfb_decoder_corre_encoding (decoder, x, y, w, h); + ret = rfb_decoder_corre_encoding (decoder, x, y, w, h); break; case ENCODING_TYPE_HEXTILE: - rfb_decoder_hextile_encoding (decoder, x, y, w, h); + ret = rfb_decoder_hextile_encoding (decoder, x, y, w, h); break; default: g_critical ("unimplemented encoding\n"); break; } + + if (!ret) + return FALSE; + decoder->n_rects--; if (decoder->n_rects == 0) { decoder->state = NULL; } else { decoder->state = rfb_decoder_state_framebuffer_update_rectangle; } + return TRUE; } -static void +static gboolean rfb_decoder_raw_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h) { @@ -817,8 +855,11 @@ rfb_decoder_raw_encoding (RfbDecoder * decoder, gint start_x, gint start_y, raw_line_size = rect_w * decoder->bytespp; size = rect_h * raw_line_size; + GST_DEBUG ("Reading %d bytes (%dx%d)", size, rect_w, rect_h); - rfb_decoder_read (decoder, size); + + if (!rfb_decoder_read (decoder, size)) + return FALSE; frame = decoder->frame + (((start_y * decoder->rect_width) + @@ -830,9 +871,11 @@ rfb_decoder_raw_encoding (RfbDecoder * decoder, gint start_x, gint start_y, p += raw_line_size; frame += decoder->line_size; } + + return TRUE; } -static void +static gboolean rfb_decoder_copyrect_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h) { @@ -840,7 +883,8 @@ rfb_decoder_copyrect_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint line_width, copyrect_width; guint8 *src, *dst; - rfb_decoder_read (decoder, 4); + if (!rfb_decoder_read (decoder, 4)) + return FALSE; /* don't forget the offset */ src_x = RFB_GET_UINT16 (decoder->data) - decoder->offset_x; @@ -861,6 +905,8 @@ rfb_decoder_copyrect_encoding (RfbDecoder * decoder, gint start_x, gint start_y, src += line_width; dst += line_width; } + + return TRUE; } static void @@ -882,14 +928,16 @@ rfb_decoder_fill_rectangle (RfbDecoder * decoder, gint x, gint y, gint w, } } -static void +static gboolean rfb_decoder_rre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h) { guint32 number_of_rectangles, color; guint16 x, y, w, h; - rfb_decoder_read (decoder, 4 + decoder->bytespp); + if (!rfb_decoder_read (decoder, 4 + decoder->bytespp)) + return FALSE; + number_of_rectangles = RFB_GET_UINT32 (decoder->data); color = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data + 4))); @@ -900,7 +948,9 @@ rfb_decoder_rre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, while (number_of_rectangles--) { - rfb_decoder_read (decoder, decoder->bytespp + 8); + if (!rfb_decoder_read (decoder, decoder->bytespp + 8)) + return FALSE; + color = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data))); x = RFB_GET_UINT16 (decoder->data + decoder->bytespp); y = RFB_GET_UINT16 (decoder->data + decoder->bytespp + 2); @@ -910,16 +960,20 @@ rfb_decoder_rre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, /* draw the rectangle in the foreground */ rfb_decoder_fill_rectangle (decoder, start_x + x, start_y + y, w, h, color); } + + return TRUE; } -static void +static gboolean rfb_decoder_corre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h) { guint32 number_of_rectangles, color; guint8 x, y, w, h; - rfb_decoder_read (decoder, 4 + decoder->bytespp); + if (!rfb_decoder_read (decoder, 4 + decoder->bytespp)) + return FALSE; + number_of_rectangles = RFB_GET_UINT32 (decoder->data); color = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data + 4))); g_free (decoder->data); @@ -931,7 +985,9 @@ rfb_decoder_corre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, while (number_of_rectangles--) { - rfb_decoder_read (decoder, decoder->bytespp + 4); + if (!rfb_decoder_read (decoder, decoder->bytespp + 4)) + return FALSE; + color = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data))); x = RFB_GET_UINT8 (decoder->data + decoder->bytespp); y = RFB_GET_UINT8 (decoder->data + decoder->bytespp + 1); @@ -943,9 +999,11 @@ rfb_decoder_corre_encoding (RfbDecoder * decoder, gint start_x, gint start_y, g_free (decoder->data); } + + return TRUE; } -static void +static gboolean rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, gint rect_w, gint rect_h) { @@ -967,7 +1025,9 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, for (y = start_y; y < y_max; y += 16) { for (x = start_x; x < x_max; x += 16) { - rfb_decoder_read (decoder, 1); + if (!rfb_decoder_read (decoder, 1)) + return FALSE; + subencoding = RFB_GET_UINT8 (decoder->data); if (subencoding & SUBENCODING_RAW) { @@ -977,7 +1037,9 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, } if (subencoding & SUBENCODING_BACKGROUND) { - rfb_decoder_read (decoder, decoder->bytespp); + if (!rfb_decoder_read (decoder, decoder->bytespp)) + return FALSE; + background = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data))); } rfb_decoder_fill_rectangle (decoder, x, y, @@ -985,12 +1047,16 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, background); if (subencoding & SUBENCODING_FOREGROUND) { - rfb_decoder_read (decoder, decoder->bytespp); + if (!rfb_decoder_read (decoder, decoder->bytespp)) + return FALSE; + foreground = GUINT32_SWAP_LE_BE ((RFB_GET_UINT32 (decoder->data))); } if (subencoding & SUBENCODING_ANYSUBRECTS) { - rfb_decoder_read (decoder, 1); + if (!rfb_decoder_read (decoder, 1)) + return FALSE; + nr_subrect = RFB_GET_UINT8 (decoder->data); } else { continue; @@ -999,7 +1065,8 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, if (subencoding & SUBENCODING_SUBRECTSCOLORED) { guint offset = 0; - rfb_decoder_read (decoder, nr_subrect * (2 + decoder->bytespp)); + if (!rfb_decoder_read (decoder, nr_subrect * (2 + decoder->bytespp))) + return FALSE; while (nr_subrect--) { foreground = @@ -1013,7 +1080,8 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, } else { guint offset = 0; - rfb_decoder_read (decoder, 2 * nr_subrect); + if (!rfb_decoder_read (decoder, 2 * nr_subrect)) + return FALSE; while (nr_subrect--) { xy = RFB_GET_UINT8 (decoder->data + offset++); @@ -1024,6 +1092,8 @@ rfb_decoder_hextile_encoding (RfbDecoder * decoder, gint start_x, gint start_y, } } } + + return TRUE; } static gboolean @@ -1040,13 +1110,18 @@ rfb_decoder_state_server_cut_text (RfbDecoder * decoder) gint cut_text_length; /* 3 bytes padding, 4 bytes cut_text_length */ - rfb_decoder_read (decoder, 7); + if (!rfb_decoder_read (decoder, 7)) + return FALSE; + cut_text_length = RFB_GET_UINT32 (decoder->data + 3); - rfb_decoder_read (decoder, cut_text_length); + if (!rfb_decoder_read (decoder, cut_text_length)) + return FALSE; + GST_DEBUG ("rfb_decoder_state_server_cut_text: throw away '%s'", decoder->data); decoder->state = rfb_decoder_state_normal; + return TRUE; }