[Spice-devel] [PATCH spice-gtk 0/2] Fix minor read buffer overflow

[Frediano Ziglio]

Avoid in some situations an malicious server to led to some minor
reading buffer overflows. These overflows cannot cause code execution
or information leakage.

Frediano Ziglio (2):
spice-channel: Check minumum size of peer_msg
spice-channel: Avoid some buffer reading overflows

src/spice-channel.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

[Frediano Ziglio]

Other parts of the code assume peer_msg contains at least a fixed
structure so make sure server is sending enough data.

Signed-off-by: Frediano Ziglio <***@redhat.com>
---
src/spice-channel.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index c61bcbab..7e5b2e7f 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1400,6 +1400,11 @@ static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
c->peer_hdr.minor_version = GUINT32_FROM_LE(c->peer_hdr.minor_version);
c->peer_hdr.size = GUINT32_FROM_LE(c->peer_hdr.size);

+ if (c->peer_hdr.size < sizeof(*c->peer_msg)) {
+ g_warning("invalid peer header size: %u", c->peer_hdr.size);
+ goto error;
+ }
+
c->peer_msg = g_malloc0(c->peer_hdr.size);
if (c->peer_msg == NULL) {
g_warning("invalid peer header size: %u", c->peer_hdr.size);

[Frediano Ziglio]

Check link message contains valid offset and array sizes.
The overflows do not produce data leaking as data are copied into
other client arrays and used only for checking limited bit arrays.
This remove possible client DoS.

Signed-off-by: Frediano Ziglio <***@redhat.com>
---
src/spice-channel.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 7e5b2e7f..cc089ebb 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1906,7 +1906,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
int rc;
uint32_t num_caps;
uint32_t num_channel_caps, num_common_caps;
- uint8_t *caps_src;
+ const uint8_t *caps_src, *caps_end;
SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;

g_return_val_if_fail(channel != NULL, FALSE);
@@ -1947,14 +1947,25 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
num_caps = num_channel_caps + num_common_caps;
CHANNEL_DEBUG(channel, "%s: %u caps", __FUNCTION__, num_caps);

+ if (c->peer_msg->caps_offset > c->peer_hdr.size) {
+ goto error;
+ }
+ caps_end = (uint8_t*)c->peer_msg + c->peer_hdr.size;
+
/* see original spice/client code: */
/* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */

caps_src = (uint8_t *)c->peer_msg + c->peer_msg->caps_offset;
+ if ((caps_end - caps_src) / sizeof(uint32_t) < num_common_caps) {
+ goto error;
+ }
CHANNEL_DEBUG(channel, "got remote common caps:");
store_caps(caps_src, num_common_caps, c->remote_common_caps);

caps_src += num_common_caps * sizeof(uint32_t);
+ if ((caps_end - caps_src) / sizeof(uint32_t) < num_channel_caps) {
+ goto error;
+ }
CHANNEL_DEBUG(channel, "got remote channel caps:");
store_caps(caps_src, num_channel_caps, c->remote_caps);

[Christophe Fergeau]

For the series,

Post by Frediano Ziglio
Avoid in some situations an malicious server to led to some minor
reading buffer overflows. These overflows cannot cause code execution
or information leakage.
spice-channel: Check minumum size of peer_msg
spice-channel: Avoid some buffer reading overflows
src/spice-channel.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
--
2.17.2
_______________________________________________
Spice-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/spice-devel