Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Commit

Permalink
nbd: Don't fail handshake on NBD_OPT_LIST descriptions
Browse files Browse the repository at this point in the history
The NBD Protocol states that NBD_REP_SERVER may set
'length > sizeof(namelen) + namelen'; in which case the rest
of the packet is a UTF-8 description of the export.  While we
don't know of any NBD servers that send this description yet,
we had better consume the data so we don't choke when we start
to talk to such a server.

Also, a (buggy/malicious) server that replies with length <
sizeof(namelen) would cause us to block waiting for bytes that
the server is not sending, and one that replies with super-huge
lengths could cause us to temporarily allocate up to 4G memory.
Sanity check things before blindly reading incorrectly.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 1460077777-31004-1-git-send-email-eblake@redhat.com
Reviewed-by: Alex Bligh <alex@alex.org.uk>
Signed-off-by: Max Reitz <mreitz@redhat.com>
  • Loading branch information
ebblake authored and XanClic committed Apr 15, 2016
1 parent e71fc0b commit 200650d
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions nbd/client.c
Expand Up @@ -192,13 +192,18 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
return -1;
}
} else if (type == NBD_REP_SERVER) {
if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "incorrect option length");
return -1;
}
if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
error_setg(errp, "failed to read option name length");
return -1;
}
namelen = be32_to_cpu(namelen);
if (len != (namelen + sizeof(namelen))) {
error_setg(errp, "incorrect option mame length");
len -= sizeof(namelen);
if (len < namelen) {
error_setg(errp, "incorrect option name length");
return -1;
}
if (namelen > 255) {
Expand All @@ -214,6 +219,20 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
return -1;
}
(*name)[namelen] = '\0';
len -= namelen;
if (len) {
char *buf = g_malloc(len + 1);
if (read_sync(ioc, buf, len) != len) {
error_setg(errp, "failed to read export description");
g_free(*name);
g_free(buf);
*name = NULL;
return -1;
}
buf[len] = '\0';
TRACE("Ignoring export description: %s", buf);
g_free(buf);
}
} else {
error_setg(errp, "Unexpected reply type %x expected %x",
type, NBD_REP_SERVER);
Expand Down

0 comments on commit 200650d

Please sign in to comment.