[Libguestfs] Re: [nbdkit PATCH] server: Fix off-by-one for maximum block_status length [CVE-XXX]
On Wed, Apr 23, 2025 at 01:10:58PM +0100, Richard W.M. Jones wrote:
On Tue, Apr 22, 2025 at 05:07:17PM -0500, Eric Blake via Libguestfs
wrote:
> Latent since the introduction of the .extents callback. The loop
> intentionally truncates to 2**32-1 bytes, but condition to end the
> loop early used > while the assertion after the loop used <=, meaning
> that the assertion can fire for any plugin that returns an extent of
> 2**32 or larger.
>
> Fixes: 26455d45 ('server: protocol: Implement Block Status
"base:allocation".', v1.11.10)
> Reported-by: Nikolay Ivanets <stenavin(a)gmail.com>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
>
> pos += length;
> - if (pos > offset + count) /* this must be the last block */
> + if (pos >= offset + count) /* this must be the last block */
> break;
AIUI ...
blocks[i].length = length = MIN (e.length, UINT32_MAX);
The extent we're sending has e.length is >= 4G, which we truncate to
UINT32_MAX == 4G-1.
pos += length;
pos += 4G-1
In the previous code:
if (pos > offset + count) /* this must be the last block */
break;
branch not taken so we reach:
assert (e.length <= length);
e.length == 4G, length == 4G-1, so we assert here.
Yep. And if compiled with NDEBUG, the assertion doesn't trigger, but
the loop could continue and try to send the client an extent that
starts after the maximum offset permitted, so even if we don't crash,
we'd be violating the NBD spec.
In the updated code, the assert is avoided, which means this branch
must now be taken, which means the client requested 'offset' == 0 &
'count' must be exactly 4G-1, I think? (Or any relative request)
if (pos >= offset + count) /* this must be the last block */
break;
The combination of user requesting len=4G-1 and the plugin answering
with a single extent >= 4G is what I tested. I still need to analyze
if there are other conditions where the off-by-one could also bite us
where we provide too many extents in reply to the user without
triggering the assertion. But it looks like the assertion can only be
triggered if the user requests len 4G-1.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org