[Libguestfs] [PATCH 04/14] nbd/client: Add safety check on chunk payload length
Vladimir Sementsov-Ogievskiy
vsementsov at virtuozzo.com
Mon Dec 6 12:33:32 UTC 2021
04.12.2021 02:15, Eric Blake wrote:
> Our existing use of structured replies either reads into a qiov capped
> at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
> NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
> checks are rather late; if we encounter a buggy (or malicious) server
> that sends a super-large payload length, we should drop the connection
> right then rather than assuming the layer on top will be careful.
> This becomes more important when we permit 64-bit lengths which are
> even more likely to have the potential for attempted denial of service
> abuse.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
> ---
> nbd/client.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index 30d5383cb195..8f137c2320bb 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -1412,6 +1412,18 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
> chunk->handle = be64_to_cpu(chunk->handle);
> chunk->length = be32_to_cpu(chunk->length);
>
> + /*
> + * Because we use BLOCK_STATUS with REQ_ONE, and cap READ requests
> + * at 32M, no valid server should send us payload larger than
> + * this. Even if we stopped using REQ_ONE, sane servers will cap
> + * the number of extents they return for block status.
> + */
> + if (chunk->length > NBD_MAX_BUFFER_SIZE + sizeof(NBDStructuredReadData)) {
> + error_setg(errp, "server chunk %" PRIu32 " (%s) payload is too long",
> + chunk->type, nbd_rep_lookup(chunk->type));
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
--
Best regards,
Vladimir
More information about the Libguestfs
mailing list