[PATCH 2/2] qemu-img: CVE-XXX Sanitize untrusted output from NBD server

[Eric Blake]

Error messages from an NBD server must be treated as untrusted; a
malicious server can inject escape sequences to try and trigger RCE
flaws via escape sequences to whatever terminal happens to be running
qemu-img.  The easiest solution is to sanitize the output with the
same code we use to produce sanitized (pseudo-)JSON over QMP.

Rich Jones originally pointed this flaw out at:
https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/thread/2NXA23G2V3HPWJYAO726PLNBEAAEUJAU/

With this patch, and a malicious server run with nbdkit 1.40 as:

$ nbdkit --log=null eval open=' printf \
  "EPERM x\\r mess up the output \e[31mmess up the output\e[m mess up" >&2; \
  exit 1 ' get_size=' echo 0 ' --run 'qemu-img info "$uri"'

we now get:

qemu-img: Could not open 'nbd://localhost': Requested export not available
server reported: /tmp/nbdkitOZHOKB/open: x\r mess up the output \u001B[31mmess 
up the output\u001B[m mess up

instead of an attempt to hide the name of the Unix socket and forcing
the terminal to render part of the text red.

Note that I did _not_ sanitize the string being sent through
trace-events in trace_nbd_server_error_msg; this is because I assume
that our trace engines already treat all string strings as untrusted
input and apply their own escaping as needed.

Reported-by: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

---

If my assumption about allowing raw escape bytes through to trace_
calls is wrong (such as when tracing to stderr), let me know.  That's
a much bigger audit to determine which trace points, if any, should
sanitize data before tracing, and/or change the trace engines to
sanitize all strings (with possible knock-on effects if trace output
changes unexpectedly for a tool expecting something unsanitized).
---
 nbd/client.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/nbd/client.c b/nbd/client.c
index c89c7504673..baa20d10d69 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -23,6 +23,7 @@
 #include "trace.h"
 #include "nbd-internal.h"
 #include "qemu/cutils.h"
+#include "qemu/unicode.h"

 /* Definitions for opaque data types */

@@ -230,7 +231,9 @@ static int nbd_handle_reply_err(QIOChannel *ioc, 
NBDOptionReply *reply,
     }

     if (msg) {
-        error_append_hint(errp, "server reported: %s\n", msg);
+        g_autoptr(GString) buf = g_string_sized_new(reply->length);
+        mod_utf8_sanitize(buf, msg);
+        error_append_hint(errp, "server reported: %s\n", buf->str);
     }

  err:
-- 
2.45.2