[Libguestfs] [PATCH nbdkit 2/2] server: Zero the read buffer before passing it to plugin .pread method.

Richard W.M. Jones rjones at redhat.com
Tue Apr 23 09:01:57 UTC 2019


If the plugin .pread method did not fill the buffer with data then the
contents of the heap could be leaked back to the client.  The simplest
way to avoid any possibility of this happening is to zero the buffer
before passing it to the .pread method.

I have checked plugins shipped with nbdkit and none of them are
vulnerable in this way.  They all do one of these things:

 - They fully set the buffer with a single call to something like
   memcpy, memset, etc.

 - They use a loop like ‘while (count > 0)’ and correctly update count
   and the buffer pointer on each iteration.

 - For language plugins (except OCaml), they check that the string
   length returned from the language plugin is at least as long as the
   requested data, before memcpy-ing the data to the return buffer.

 - For OCaml, see the previous commit.

Of course I cannot check plugins which may be supplied by others.

Credit: Eric Blake for finding the bug.
---
 server/protocol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/server/protocol.c b/server/protocol.c
index 54d8adb..4d67392 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -658,10 +658,10 @@ protocol_recv_request_send_reply (struct connection *conn)
 
     /* Allocate the data buffer used for either read or write requests. */
     if (cmd == NBD_CMD_READ || cmd == NBD_CMD_WRITE) {
-      buf = malloc (count);
+      buf = calloc (1, count);
       if (buf == NULL) {
       out_of_memory:
-        perror ("malloc");
+        perror ("calloc");
         error = ENOMEM;
         if (cmd == NBD_CMD_WRITE &&
             skip_over_write_buffer (conn->sockin, count) < 0)
-- 
2.20.1




More information about the Libguestfs mailing list