[nbdkit PATCH] server: Fix off-by-one for maximum block_status length [CVE-XXX]
by Eric Blake
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>
---
We still need to sort out if this gets a CVE number; either way, the
commit title needs an update, and if it does get a CVE we need to also
post a security alert message to the list.
tests/Makefile.am | 2 ++
server/protocol.c | 2 +-
tests/test-eval-extents.sh | 60 ++++++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 1 deletion(-)
create mode 100755 tests/test-eval-extents.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 13c0457c..ffe42c78 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -881,6 +881,7 @@ TESTS += \
test-eval.sh \
test-eval-file.sh \
test-eval-exports.sh \
+ test-eval-extents.sh \
test-eval-cache.sh \
test-eval-dump-plugin.sh \
test-eval-disconnect.sh \
@@ -889,6 +890,7 @@ EXTRA_DIST += \
test-eval.sh \
test-eval-file.sh \
test-eval-exports.sh \
+ test-eval-extents.sh \
test-eval-cache.sh \
test-eval-dump-plugin.sh \
test-eval-disconnect.sh \
diff --git a/server/protocol.c b/server/protocol.c
index d428bfc8..b4b1c162 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -499,7 +499,7 @@ extents_to_block_descriptors (struct nbdkit_extents *extents,
(*nr_blocks)++;
pos += length;
- if (pos > offset + count) /* this must be the last block */
+ if (pos >= offset + count) /* this must be the last block */
break;
/* If we reach here then we must have consumed this whole
diff --git a/tests/test-eval-extents.sh b/tests/test-eval-extents.sh
new file mode 100755
index 00000000..08c8d670
--- /dev/null
+++ b/tests/test-eval-extents.sh
@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright Red Hat
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+source ./functions.sh
+set -e
+set -x
+
+requires_run
+requires_plugin eval
+requires_nbdsh_uri
+requires nbdsh --base-allocation
+
+files="eval-extents.out"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Trigger an off-by-one bug introduced in v1.11.10 and fixed in v1.43.6
+export script='
+def f(context, offset, extents, status):
+ print(extents)
+
+h.block_status(2**32-1, 0, f)
+'
+nbdkit eval \
+ get_size='echo 5G' \
+ pread='dd if=/dev/zero count=$3 iflag=count_bytes' \
+ extents='echo 0 5G 0' \
+ --run 'nbdsh --base-allocation --uri "$uri" -c "$script"' \
+ > eval-extents.out
+cat eval-extents.out
+grep -E '\[4294967295, 0]' eval-extents.out
--
2.49.0
4 hours, 16 minutes
nbdkit crashes on attempt to retrieve extent with len of 2^32-1
by Nikolay Ivanets
nbdkit crashes when the client is trying to get extents with len=2^32-1.
Here is client code (nbdsh):
h.add_meta_context("base:allocation")
h.connect_uri("nbd://localhost:10809/disk0-flat.vmdk")
def f(metacontext, offset, e, status):
print(e)
h.block_status(2**32-2, 0, f)
[4294967295, 0] <--- OK
h.block_status(2**32-1, 0, f) <-- FAIL
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/usr/lib64/python3.12/site-packages/nbd.py", line 2775, in
block_status
return libnbdmod.block_status(self._o, count, offset, extent, flags)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nbd.Error: nbd_block_status: block-status: command failed: Transport
endpoint is not connected (ENOTCONN)
Server prints:
nbdkit: file.8: debug: file: extents count=4294967295 offset=0 req_one=0
nbdkit: ../../server/protocol.c:505: extents_to_block_descriptors:
Assertion `e.length <= length' failed.
Aborted (core dumped)
Why 2^32-2 is max len, and why nbdkit crashes with 2**32-1? It seems like a
bad-behaviour client can crash the server. Or did I miss something?
--
+380979184774
Mykola Ivanets
22 hours, 15 minutes
