On 1/20/20 6:28 AM, Vladimir Sementsov-Ogievskiy wrote:
As far as I can see, NBD just passes NBDRequest.from (which is a
uint64_t) to this function (on NBD_CMD_BLOCK_STATUS). Would this allow
a malicious client to send a value > INT64_MAX, thus provoking an
overflow and killing the server with this new assertion?
in nbd_co_receive_request() we have
if (request->from > client->exp->size ||
request->len > client->exp->size - request->from) {
So, we check that from is <= exp->size. and exp->size cant be greater than
INT64_MAX,
as it derived from bdrv_getlength, which returns int64_t.
Interesting, should we be more strict in server:?
I think we're okay based on the existing bounds checks.
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2178,7 +2178,7 @@ static int nbd_co_receive_request(NBDRequestData *req,
NBDRequest *request,
error_setg(errp, "Export is read-only");
return -EROFS;
}
- if (request->from > client->exp->size ||
+ if (request->from >= client->exp->size ||
request->len > client->exp->size - request->from) {
error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %"
PRIu32
", Size: %" PRIu64, request->from, request->len,
Or is it intentional? Looking through NBD spec I found only
client MUST NOT use a length ... or which, when added to offset, would
exceed the export size.
So, formally pair offset=<export size>, len=0 is valid...
Except that the spec also says that len=0 is generally unspecified
behavior (whether it is a no-op, or means special handling, or whatever
else, is up to the server, but clients shouldn't be sending it - thus a
server that rejects it instead of handling it as a no-op is no worse for
the wear).
On second thought, we have this problem already everywhere in
nbd_handle_request(). I don’t see it or its caller ever checking
whether the received values are in bounds, it just passes them to all
kind of block layer functions that sometimes even just accept plain
ints. Well, I suppose all other functions just error out, so it
probably isn’t an actual problem in practice so far...
Max
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
