From 15f6c4f3dc51c68e0f869fb13c75c79bd2f0a4db Mon Sep 17 00:00:00 2001 From: itsankoff Date: Tue, 24 Oct 2017 20:21:03 +0300 Subject: [PATCH 1/6] GUACAMOLE-424: Fix null pointer dereference for vnc client display --- src/protocols/vnc/user.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/protocols/vnc/user.c b/src/protocols/vnc/user.c index 7e123732..b036785f 100644 --- a/src/protocols/vnc/user.c +++ b/src/protocols/vnc/user.c @@ -112,8 +112,10 @@ int guac_vnc_user_leave_handler(guac_user* user) { guac_vnc_client* vnc_client = (guac_vnc_client*) user->client->data; - /* Update shared cursor state */ - guac_common_cursor_remove_user(vnc_client->display->cursor, user); + if (vnc_client && vnc_client->display && vnc_client->display->cursor) { + /* Update shared cursor state */ + guac_common_cursor_remove_user(vnc_client->display->cursor, user); + } /* Free settings if not owner (owner settings will be freed with client) */ if (!user->owner) { From bbafa00df0df242ac308462e16e78c4c23e88a96 Mon Sep 17 00:00:00 2001 From: itsankoff Date: Thu, 2 Nov 2017 19:53:17 +0200 Subject: [PATCH 2/6] GUACAMOLE-424: Prevent null pointer dereference for vnc client display and cursor --- src/protocols/vnc/user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocols/vnc/user.c b/src/protocols/vnc/user.c index b036785f..ea09eb09 100644 --- a/src/protocols/vnc/user.c +++ b/src/protocols/vnc/user.c @@ -112,7 +112,7 @@ int guac_vnc_user_leave_handler(guac_user* user) { guac_vnc_client* vnc_client = (guac_vnc_client*) user->client->data; - if (vnc_client && vnc_client->display && vnc_client->display->cursor) { + if (vnc_client->display && vnc_client->display->cursor) { /* Update shared cursor state */ guac_common_cursor_remove_user(vnc_client->display->cursor, user); } From da0fc1a6d8c2296eec2c43ca06cb9f37df18d9f7 Mon Sep 17 00:00:00 2001 From: itsankoff Date: Mon, 13 Nov 2017 14:49:44 +0200 Subject: [PATCH 3/6] GUACAMOLE-424: Add doc comment for guac_common_cursor_alloc --- src/common/cursor.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/common/cursor.c b/src/common/cursor.c index f956f6aa..616f23b5 100644 --- a/src/common/cursor.c +++ b/src/common/cursor.c @@ -34,6 +34,14 @@ #include #include + +/** + * Allocates a cursor as well as an image buffer where the cursor is rendered. + * If the allocation fails then the function returns NULL. + * + * @param client + * The client owning the cursor. + */ guac_common_cursor* guac_common_cursor_alloc(guac_client* client) { guac_common_cursor* cursor = malloc(sizeof(guac_common_cursor)); From f7990af6d0c6aac58b705049e52c2587c21b6596 Mon Sep 17 00:00:00 2001 From: itsankoff Date: Mon, 13 Nov 2017 14:50:44 +0200 Subject: [PATCH 4/6] GUACAMOLE-424: Return NULL if guac_common_display allocation fails --- src/common/display.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/common/display.c b/src/common/display.c index 09bf67e7..ff70c587 100644 --- a/src/common/display.c +++ b/src/common/display.c @@ -99,6 +99,19 @@ static void guac_common_display_free_layers(guac_common_display_layer* layers, } +/** + * Allocates a display and a cursor which are used to represent the remote + * display and cursor. If the allocation fails then the function returns NULL. + * + * @param client + * The client owning the cursor. + * + * @param width + * The desired width of the display. + * + * @param height + * The desired height of the display. + */ guac_common_display* guac_common_display_alloc(guac_client* client, int width, int height) { @@ -107,14 +120,18 @@ guac_common_display* guac_common_display_alloc(guac_client* client, if (display == NULL) return NULL; + /* Allocate shared cursor */ + display->cursor = guac_common_cursor_alloc(client); + if (display->cursor == NULL) { + free(display); + return NULL; + } + pthread_mutex_init(&display->_lock, NULL); /* Associate display with given client */ display->client = client; - /* Allocate shared cursor */ - display->cursor = guac_common_cursor_alloc(client); - display->default_surface = guac_common_surface_alloc(client, client->socket, GUAC_DEFAULT_LAYER, width, height); From e139b20d12f7dccf06b08494e1154735b2f12510 Mon Sep 17 00:00:00 2001 From: itsankoff Date: Mon, 13 Nov 2017 14:51:31 +0200 Subject: [PATCH 5/6] GUACAMOLE-424: Remove check against NULL for display cursor --- src/protocols/vnc/user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocols/vnc/user.c b/src/protocols/vnc/user.c index ea09eb09..da3b843f 100644 --- a/src/protocols/vnc/user.c +++ b/src/protocols/vnc/user.c @@ -112,7 +112,7 @@ int guac_vnc_user_leave_handler(guac_user* user) { guac_vnc_client* vnc_client = (guac_vnc_client*) user->client->data; - if (vnc_client->display && vnc_client->display->cursor) { + if (vnc_client->display) { /* Update shared cursor state */ guac_common_cursor_remove_user(vnc_client->display->cursor, user); } From aa6d81d6f944df6654c74ab4b514b1fa388c4850 Mon Sep 17 00:00:00 2001 From: itsankoff Date: Wed, 15 Nov 2017 14:58:06 +0200 Subject: [PATCH 6/6] GUACAMOLE-424: Update doc comments --- src/common/cursor.c | 4 +++- src/common/display.c | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/common/cursor.c b/src/common/cursor.c index 616f23b5..f621b942 100644 --- a/src/common/cursor.c +++ b/src/common/cursor.c @@ -37,10 +37,12 @@ /** * Allocates a cursor as well as an image buffer where the cursor is rendered. - * If the allocation fails then the function returns NULL. * * @param client * The client owning the cursor. + * + * @return + * The newly-allocated cursor or NULL if cursor cannot be allocated. */ guac_common_cursor* guac_common_cursor_alloc(guac_client* client) { diff --git a/src/common/display.c b/src/common/display.c index ff70c587..5d8ce9f1 100644 --- a/src/common/display.c +++ b/src/common/display.c @@ -101,7 +101,7 @@ static void guac_common_display_free_layers(guac_common_display_layer* layers, /** * Allocates a display and a cursor which are used to represent the remote - * display and cursor. If the allocation fails then the function returns NULL. + * display and cursor. * * @param client * The client owning the cursor. @@ -111,6 +111,9 @@ static void guac_common_display_free_layers(guac_common_display_layer* layers, * * @param height * The desired height of the display. + * + * @return + * The newly-allocated display or NULL if display cannot be allocated. */ guac_common_display* guac_common_display_alloc(guac_client* client, int width, int height) {