On 8 Apr 2016, at 23:05, Eric Blake <ebl...@redhat.com> wrote:
> 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 <ebl...@redhat.com>
Reviewed-by: Alex Bligh <a...@alex.org.uk>
> ---
> nbd/client.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index 6777e58..48f2a21 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -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) {
> @@ -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);
> --
> 2.5.5
>
>
--
Alex Bligh
