From 69cb53266f5b2536e22a4ece7a5eacb6e5ec69d9 Mon Sep 17 00:00:00 2001 From: Jesse Bakker Date: Mon, 13 Feb 2023 17:09:33 +0100 Subject: [PATCH 1/2] Add support for RFB protocol version 3.7 and 3.8 --- vnc/vnc.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 93 insertions(+), 12 deletions(-) diff --git a/vnc/vnc.c b/vnc/vnc.c index e6b84ace12..7b749eebb6 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -1643,10 +1643,15 @@ lib_mod_connect(struct vnc *v) char cursor_mask[32 * (32 / 8)]; char con_port[256]; char text[256]; + char *version_str; + char j; struct stream *s; struct stream *pixel_format; int error; int i; + int sec_lvl = 0; + int version; + int n_sec_lvls; int check_sec_result; v->server_msg(v, "VNC started connecting", 0); @@ -1710,8 +1715,27 @@ lib_mod_connect(struct vnc *v) error = trans_force_read_s(v->trans, s, 12); if (error == 0) { - s->p = s->data; - out_uint8a(s, "RFB 003.003\n", 12); + in_uint8p(s, version_str, 12); + if (g_strncmp(version_str, "RFB 003.00", 10) != 0) + { + version_str[11] = '\0'; + LOG(LOG_LEVEL_ERROR, "Invalid server version string %s", version_str); + error = 1; + } + } + if (error == 0) + { + version = version_str[10] - '0'; + if (version != 3 && version != 7 && version != 8) + { + LOG(LOG_LEVEL_ERROR, "Unsupported VNC version 3.%d", version); + error = 1; + } + } + if (error == 0) + { + init_stream(s, 8192); + out_uint8p(s, version_str, 12); s_mark_end(s); error = trans_force_write_s(v->trans, s); } @@ -1720,20 +1744,78 @@ lib_mod_connect(struct vnc *v) if (error == 0) { init_stream(s, 8192); - error = trans_force_read_s(v->trans, s, 4); + if (version == 3) + { + // The server chooses the security type + error = trans_force_read_s(v->trans, s, 4); + if (error == 0) + { + in_uint32_be(s, sec_lvl); + } + } + else if (version >= 7) + { + // The client chooses the security type + error = trans_force_read_s(v->trans, s, 1); + if (error == 0) + { + in_uint8(s, n_sec_lvls); + if (n_sec_lvls > 0) + { + error = trans_force_read_s(v->trans, s, n_sec_lvls); + } + else + { + // Read size of reason + error = trans_force_read_s(v->trans, s, 4); + if (error == 0) + { + in_uint32_be(s, i); + error = trans_force_read_s(v->trans, s, i); + in_uint8a(s, text, i); + text[i] = '\0'; + LOG(LOG_LEVEL_ERROR, "Connection closed by server with reason: %s", text); + error = 1; + } + } + } + if (error == 0) + { + for (; n_sec_lvls > 0; --n_sec_lvls) + { + in_uint8(s, j); + // Choose the highest security level that we support + if (j > sec_lvl && j <= 2) + { + sec_lvl = j; + } + } + init_stream(s, 8192); + out_uint8(s, sec_lvl); + s_mark_end(s); + + error = trans_force_write_s(v->trans, s); + } + } } if (error == 0) { - in_uint32_be(s, i); - g_sprintf(text, "VNC security level is %d (1 = none, 2 = standard)", i); + g_sprintf(text, "VNC security level is %d (1 = none, 2 = standard)", sec_lvl); v->server_msg(v, text, 0); - if (i == 1) /* none */ + if (sec_lvl == 1) /* none */ { - check_sec_result = 0; + if (version >= 8) + { + check_sec_result = 1; + } + else + { + check_sec_result = 0; + } } - else if (i == 2) /* dec the password and the server random */ + else if (sec_lvl == 2) /* dec the password and the server random */ { init_stream(s, 8192); error = trans_force_read_s(v->trans, s, 16); @@ -1757,14 +1839,14 @@ lib_mod_connect(struct vnc *v) check_sec_result = 1; // not needed } } - else if (i == 0) + else if (sec_lvl == 0) { LOG(LOG_LEVEL_ERROR, "VNC Server will disconnect"); error = 1; } else { - LOG(LOG_LEVEL_ERROR, "VNC unsupported security level %d", i); + LOG(LOG_LEVEL_ERROR, "VNC unsupported security level %d", sec_lvl); error = 1; } } @@ -1802,8 +1884,7 @@ lib_mod_connect(struct vnc *v) { v->server_msg(v, "VNC sending share flag", 0); init_stream(s, 8192); - s->data[0] = 1; - s->p++; + out_uint8(s, 1); s_mark_end(s); error = trans_force_write_s(v->trans, s); /* share flag */ } From 519264025981b4259f83db0ac606061fe01bd47d Mon Sep 17 00:00:00 2001 From: Jesse Bakker Date: Tue, 28 Feb 2023 12:03:59 +0100 Subject: [PATCH 2/2] Review fixes --- common/parse.h | 9 +++ vnc/vnc.c | 162 +++++++++++++++++++++++++++++++++---------------- 2 files changed, 118 insertions(+), 53 deletions(-) diff --git a/common/parse.h b/common/parse.h index a3721ef89f..9bec6b607c 100644 --- a/common/parse.h +++ b/common/parse.h @@ -440,6 +440,15 @@ parser_stream_overflow_check(const struct stream *s, int n, int is_out, (s)->p += (n); \ } while (0) +/******************************************************************************/ +#define in_str(s, v, n) do \ + { \ + S_CHECK_REM((s), (n + 1)); \ + (v) = (s)->p; \ + (v[n]) = '\0'; \ + (s)->p++; \ + } while (0) + /******************************************************************************/ #define in_uint8a(s, v, n) do \ { \ diff --git a/vnc/vnc.c b/vnc/vnc.c index 7b749eebb6..1b3a9c3264 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -1643,7 +1643,8 @@ lib_mod_connect(struct vnc *v) char cursor_mask[32 * (32 / 8)]; char con_port[256]; char text[256]; - char *version_str; + char version_str[12]; + char *err_reason; char j; struct stream *s; struct stream *pixel_format; @@ -1715,13 +1716,18 @@ lib_mod_connect(struct vnc *v) error = trans_force_read_s(v->trans, s, 12); if (error == 0) { - in_uint8p(s, version_str, 12); + in_uint8a(s, version_str, 12); if (g_strncmp(version_str, "RFB 003.00", 10) != 0) { version_str[11] = '\0'; LOG(LOG_LEVEL_ERROR, "Invalid server version string %s", version_str); error = 1; } + if (version_str[11] != '\n') + { + LOG(LOG_LEVEL_ERROR, "Invalid server version string (missing trailing newline)"); + error = 1; + } } if (error == 0) { @@ -1771,10 +1777,28 @@ lib_mod_connect(struct vnc *v) if (error == 0) { in_uint32_be(s, i); + if (i <= 1024) + { + error = trans_force_read_s(v->trans, s, i); + } + else + { + LOG(LOG_LEVEL_ERROR, "Connection closed by server. Reason string exceeds size limit (%d > 1024 bytes)", i); + error = 1; + } + } + else + { + i = 0; + } + if (error == 0) + { error = trans_force_read_s(v->trans, s, i); - in_uint8a(s, text, i); - text[i] = '\0'; - LOG(LOG_LEVEL_ERROR, "Connection closed by server with reason: %s", text); + } + if (error == 0) + { + in_str(s, err_reason, i); + LOG(LOG_LEVEL_ERROR, "Connection closed by server with reason: %s", err_reason); error = 1; } } @@ -1790,66 +1814,69 @@ lib_mod_connect(struct vnc *v) sec_lvl = j; } } - init_stream(s, 8192); - out_uint8(s, sec_lvl); - s_mark_end(s); + if (sec_lvl != 0) + { + init_stream(s, 8192); + out_uint8(s, sec_lvl); + s_mark_end(s); - error = trans_force_write_s(v->trans, s); + error = trans_force_write_s(v->trans, s); + } } } } + } - if (error == 0) + if (error == 0) + { + g_sprintf(text, "VNC security level is %d (1 = none, 2 = standard)", sec_lvl); + v->server_msg(v, text, 0); + + if (sec_lvl == 1) /* none */ + { + if (version >= 8) + { + check_sec_result = 1; + } + else + { + check_sec_result = 0; + } + } + else if (sec_lvl == 2) /* dec the password and the server random */ { - g_sprintf(text, "VNC security level is %d (1 = none, 2 = standard)", sec_lvl); - v->server_msg(v, text, 0); + init_stream(s, 8192); + error = trans_force_read_s(v->trans, s, 16); - if (sec_lvl == 1) /* none */ + if (error == 0) { - if (version >= 8) + init_stream(s, 8192); + if (guid_is_set(&v->guid)) { - check_sec_result = 1; + char guid_str[GUID_STR_SIZE]; + guid_to_str(&v->guid, guid_str); + rfbHashEncryptBytes(s->data, guid_str); } else { - check_sec_result = 0; + rfbEncryptBytes(s->data, v->password); } - } - else if (sec_lvl == 2) /* dec the password and the server random */ - { - init_stream(s, 8192); - error = trans_force_read_s(v->trans, s, 16); - - if (error == 0) - { - init_stream(s, 8192); - if (guid_is_set(&v->guid)) - { - char guid_str[GUID_STR_SIZE]; - guid_to_str(&v->guid, guid_str); - rfbHashEncryptBytes(s->data, guid_str); - } - else - { - rfbEncryptBytes(s->data, v->password); - } - s->p += 16; - s_mark_end(s); - error = trans_force_write_s(v->trans, s); - check_sec_result = 1; // not needed - } - } - else if (sec_lvl == 0) - { - LOG(LOG_LEVEL_ERROR, "VNC Server will disconnect"); - error = 1; - } - else - { - LOG(LOG_LEVEL_ERROR, "VNC unsupported security level %d", sec_lvl); - error = 1; + s->p += 16; + s_mark_end(s); + error = trans_force_write_s(v->trans, s); + check_sec_result = 1; // not needed } } + else if (sec_lvl == 0) + { + LOG(LOG_LEVEL_ERROR, "VNC Server will disconnect"); + error = 1; + } + else + { + LOG(LOG_LEVEL_ERROR, "VNC unsupported security level %d", sec_lvl); + error = 1; + } } if (error != 0) @@ -1870,8 +1897,37 @@ lib_mod_connect(struct vnc *v) if (i != 0) { - v->server_msg(v, "VNC password failed", 0); - error = 2; + if (version >= 8) + { + error = trans_force_read_s(v->trans, s, 4); + if (error == 0) + { + in_uint32_be(s, i); + if (i <= 1024) + { + error = trans_force_read_s(v->trans, s, i); + } + else + { + LOG(LOG_LEVEL_ERROR, "VNC security failure reason exceeds size limit (%d > 1024 bytes)", i); + error = 1; + } + } + else + { + i = 0; + } + if (error == 0) + { + in_str(s, err_reason, i); + v->server_msg(v, err_reason, 0); + } + } + if (version < 8 || error != 0) + { + v->server_msg(v, "VNC password failed", 0); + error = 2; + } } else { @@ -2154,7 +2210,7 @@ init_client_layout(struct vnc_screen_layout *layout, for (i = 0 ; i < client_info->display_sizes.monitorCount ; ++i) { /* Use minfo_wm, as this is normalised for a top-left of (0,0) - * as required by RFC6143 */ + * as required by RFC6143 */ layout->s[i].id = i; layout->s[i].x = client_info->display_sizes.minfo_wm[i].left; layout->s[i].y = client_info->display_sizes.minfo_wm[i].top;