aboutsummaryrefslogtreecommitdiff
path: root/qga/commands.c
diff options
context:
space:
mode:
authorPhilippe Mathieu-Daudé <philmd@redhat.com>2020-04-14 15:30:44 +0200
committerMichael Roth <mdroth@linux.vnet.ibm.com>2020-04-15 09:18:48 -0500
commit1329651fb4d4c5068ad12fd86aff7e52f9e18c34 (patch)
treee2f2360f075a9666d68546a5bd01f4534072b91e /qga/commands.c
parentead83a136d54f7faa315922aff26fa11d216909f (diff)
qga: Restrict guest-file-read count to 48 MB to avoid crashes
On [*] Daniel Berrangé commented: The QEMU guest agent protocol is not sensible way to access huge files inside the guest. It requires the inefficient process of reading the entire data into memory than duplicating it again in base64 format, and then copying it again in the JSON serializer / monitor code. For arbitrary general purpose file access, especially for large files, use a real file transfer program or use a network block device, not the QEMU guest agent. To avoid bug reports as BZ#1594054 (CVE-2018-12617), follow his suggestion to put a low, hard limit on "count" in the guest agent QAPI schema, and don't allow count to be larger than 48 MB. [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg693176.html Fixes: CVE-2018-12617 Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1594054 Reported-by: Fakhri Zulkifli <mohdfakhrizulkifli@gmail.com> Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> *update schema documentation to indicate 48MB limit instead of 10MB Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Diffstat (limited to 'qga/commands.c')
-rw-r--r--qga/commands.c9
1 files changed, 8 insertions, 1 deletions
diff --git a/qga/commands.c b/qga/commands.c
index 5611117372..efc8b90281 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -11,6 +11,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "guest-agent-core.h"
#include "qga-qapi-commands.h"
#include "qapi/error.h"
@@ -24,6 +25,12 @@
#define GUEST_EXEC_MAX_OUTPUT (16*1024*1024)
/* Allocation and I/O buffer for reading guest-exec out_data/err_data - 4KB */
#define GUEST_EXEC_IO_SIZE (4*1024)
+/*
+ * Maximum file size to read - 48MB
+ *
+ * (48MB + Base64 3:4 overhead = JSON parser 64 MB limit)
+ */
+#define GUEST_FILE_READ_COUNT_MAX (48 * MiB)
/* Note: in some situations, like with the fsfreeze, logging may be
* temporarilly disabled. if it is necessary that a command be able
@@ -560,7 +567,7 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
}
if (!has_count) {
count = QGA_READ_COUNT_DEFAULT;
- } else if (count < 0 || count >= UINT32_MAX) {
+ } else if (count < 0 || count > GUEST_FILE_READ_COUNT_MAX) {
error_setg(errp, "value '%" PRId64 "' is invalid for argument count",
count);
return NULL;