|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH 04/14] nbd/client: Add safety check on chunk payload length |
Date: | Mon, 6 Dec 2021 15:33:32 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
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@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@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
[Prev in Thread] | Current Thread | [Next in Thread] |