diff options
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu.inc | 1 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2020-10761.patch | 151 |
2 files changed, 152 insertions, 0 deletions
diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc index 6b9dd99ec3..d41cc8f200 100644 --- a/meta/recipes-devtools/qemu/qemu.inc +++ b/meta/recipes-devtools/qemu/qemu.inc | |||
| @@ -31,6 +31,7 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \ | |||
| 31 | file://0001-qemu-Do-not-include-file-if-not-exists.patch \ | 31 | file://0001-qemu-Do-not-include-file-if-not-exists.patch \ |
| 32 | file://CVE-2020-13361.patch \ | 32 | file://CVE-2020-13361.patch \ |
| 33 | file://find_datadir.patch \ | 33 | file://find_datadir.patch \ |
| 34 | file://CVE-2020-10761.patch \ | ||
| 34 | " | 35 | " |
| 35 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar" | 36 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar" |
| 36 | 37 | ||
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2020-10761.patch b/meta/recipes-devtools/qemu/qemu/CVE-2020-10761.patch new file mode 100644 index 0000000000..19f26ae5b0 --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2020-10761.patch | |||
| @@ -0,0 +1,151 @@ | |||
| 1 | From 5c4fe018c025740fef4a0a4421e8162db0c3eefd Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Eric Blake <eblake@redhat.com> | ||
| 3 | Date: Mon, 8 Jun 2020 13:26:37 -0500 | ||
| 4 | Subject: [PATCH] nbd/server: Avoid long error message assertions | ||
| 5 | CVE-2020-10761 | ||
| 6 | |||
| 7 | Ever since commit 36683283 (v2.8), the server code asserts that error | ||
| 8 | strings sent to the client are well-formed per the protocol by not | ||
| 9 | exceeding the maximum string length of 4096. At the time the server | ||
| 10 | first started sending error messages, the assertion could not be | ||
| 11 | triggered, because messages were completely under our control. | ||
| 12 | However, over the years, we have added latent scenarios where a client | ||
| 13 | could trigger the server to attempt an error message that would | ||
| 14 | include the client's information if it passed other checks first: | ||
| 15 | |||
| 16 | - requesting NBD_OPT_INFO/GO on an export name that is not present | ||
| 17 | (commit 0cfae925 in v2.12 echoes the name) | ||
| 18 | |||
| 19 | - requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is | ||
| 20 | not present (commit e7b1948d in v2.12 echoes the name) | ||
| 21 | |||
| 22 | At the time, those were still safe because we flagged names larger | ||
| 23 | than 256 bytes with a different message; but that changed in commit | ||
| 24 | 93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD | ||
| 25 | string limit. (That commit also failed to change the magic number | ||
| 26 | 4096 in nbd_negotiate_send_rep_err to the just-introduced named | ||
| 27 | constant.) So with that commit, long client names appended to server | ||
| 28 | text can now trigger the assertion, and thus be used as a denial of | ||
| 29 | service attack against a server. As a mitigating factor, if the | ||
| 30 | server requires TLS, the client cannot trigger the problematic paths | ||
| 31 | unless it first supplies TLS credentials, and such trusted clients are | ||
| 32 | less likely to try to intentionally crash the server. | ||
| 33 | |||
| 34 | We may later want to further sanitize the user-supplied strings we | ||
| 35 | place into our error messages, such as scrubbing out control | ||
| 36 | characters, but that is less important to the CVE fix, so it can be a | ||
| 37 | later patch to the new nbd_sanitize_name. | ||
| 38 | |||
| 39 | Consideration was given to changing the assertion in | ||
| 40 | nbd_negotiate_send_rep_verr to instead merely log a server error and | ||
| 41 | truncate the message, to avoid leaving a latent path that could | ||
| 42 | trigger a future CVE DoS on any new error message. However, this | ||
| 43 | merely complicates the code for something that is already (correctly) | ||
| 44 | flagging coding errors, and now that we are aware of the long message | ||
| 45 | pitfall, we are less likely to introduce such errors in the future, | ||
| 46 | which would make such error handling dead code. | ||
| 47 | |||
| 48 | Reported-by: Xueqiang Wei <xuwei@redhat.com> | ||
| 49 | CC: qemu-stable@nongnu.org | ||
| 50 | Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761 | ||
| 51 | Fixes: 93676c88d7 | ||
| 52 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 53 | Message-Id: <20200610163741.3745251-2-eblake@redhat.com> | ||
| 54 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
| 55 | |||
| 56 | Upstream-Status: Backport [https://github.com/qemu/qemu/commit/5c4fe018c025740fef4a0a4421e8162db0c3eefd] | ||
| 57 | CVE: CVE-2020-10761 | ||
| 58 | Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com> | ||
| 59 | |||
| 60 | --- | ||
| 61 | nbd/server.c | 23 ++++++++++++++++++++--- | ||
| 62 | tests/qemu-iotests/143 | 4 ++++ | ||
| 63 | tests/qemu-iotests/143.out | 2 ++ | ||
| 64 | 3 files changed, 26 insertions(+), 3 deletions(-) | ||
| 65 | |||
| 66 | diff --git a/nbd/server.c b/nbd/server.c | ||
| 67 | index 02b1ed08014..20754e9ebc3 100644 | ||
| 68 | --- a/nbd/server.c | ||
| 69 | +++ b/nbd/server.c | ||
| 70 | @@ -217,7 +217,7 @@ nbd_negotiate_send_rep_verr(NBDClient *client, uint32_t type, | ||
| 71 | |||
| 72 | msg = g_strdup_vprintf(fmt, va); | ||
| 73 | len = strlen(msg); | ||
| 74 | - assert(len < 4096); | ||
| 75 | + assert(len < NBD_MAX_STRING_SIZE); | ||
| 76 | trace_nbd_negotiate_send_rep_err(msg); | ||
| 77 | ret = nbd_negotiate_send_rep_len(client, type, len, errp); | ||
| 78 | if (ret < 0) { | ||
| 79 | @@ -231,6 +231,19 @@ nbd_negotiate_send_rep_verr(NBDClient *client, uint32_t type, | ||
| 80 | return 0; | ||
| 81 | } | ||
| 82 | |||
| 83 | +/* | ||
| 84 | + * Return a malloc'd copy of @name suitable for use in an error reply. | ||
| 85 | + */ | ||
| 86 | +static char * | ||
| 87 | +nbd_sanitize_name(const char *name) | ||
| 88 | +{ | ||
| 89 | + if (strnlen(name, 80) < 80) { | ||
| 90 | + return g_strdup(name); | ||
| 91 | + } | ||
| 92 | + /* XXX Should we also try to sanitize any control characters? */ | ||
| 93 | + return g_strdup_printf("%.80s...", name); | ||
| 94 | +} | ||
| 95 | + | ||
| 96 | /* Send an error reply. | ||
| 97 | * Return -errno on error, 0 on success. */ | ||
| 98 | static int GCC_FMT_ATTR(4, 5) | ||
| 99 | @@ -595,9 +608,11 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp) | ||
| 100 | |||
| 101 | exp = nbd_export_find(name); | ||
| 102 | if (!exp) { | ||
| 103 | + g_autofree char *sane_name = nbd_sanitize_name(name); | ||
| 104 | + | ||
| 105 | return nbd_negotiate_send_rep_err(client, NBD_REP_ERR_UNKNOWN, | ||
| 106 | errp, "export '%s' not present", | ||
| 107 | - name); | ||
| 108 | + sane_name); | ||
| 109 | } | ||
| 110 | |||
| 111 | /* Don't bother sending NBD_INFO_NAME unless client requested it */ | ||
| 112 | @@ -995,8 +1010,10 @@ static int nbd_negotiate_meta_queries(NBDClient *client, | ||
| 113 | |||
| 114 | meta->exp = nbd_export_find(export_name); | ||
| 115 | if (meta->exp == NULL) { | ||
| 116 | + g_autofree char *sane_name = nbd_sanitize_name(export_name); | ||
| 117 | + | ||
| 118 | return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp, | ||
| 119 | - "export '%s' not present", export_name); | ||
| 120 | + "export '%s' not present", sane_name); | ||
| 121 | } | ||
| 122 | |||
| 123 | ret = nbd_opt_read(client, &nb_queries, sizeof(nb_queries), errp); | ||
| 124 | diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143 | ||
| 125 | index f649b361950..d2349903b1b 100755 | ||
| 126 | --- a/tests/qemu-iotests/143 | ||
| 127 | +++ b/tests/qemu-iotests/143 | ||
| 128 | @@ -58,6 +58,10 @@ _send_qemu_cmd $QEMU_HANDLE \ | ||
| 129 | $QEMU_IO_PROG -f raw -c quit \ | ||
| 130 | "nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \ | ||
| 131 | | _filter_qemu_io | _filter_nbd | ||
| 132 | +# Likewise, with longest possible name permitted in NBD protocol | ||
| 133 | +$QEMU_IO_PROG -f raw -c quit \ | ||
| 134 | + "nbd+unix:///$(printf %4096d 1 | tr ' ' a)?socket=$SOCK_DIR/nbd" 2>&1 \ | ||
| 135 | + | _filter_qemu_io | _filter_nbd | sed 's/aaaa*aa/aa--aa/' | ||
| 136 | |||
| 137 | _send_qemu_cmd $QEMU_HANDLE \ | ||
| 138 | "{ 'execute': 'quit' }" \ | ||
| 139 | diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out | ||
| 140 | index 1f4001c6013..fc9c0a761fa 100644 | ||
| 141 | --- a/tests/qemu-iotests/143.out | ||
| 142 | +++ b/tests/qemu-iotests/143.out | ||
| 143 | @@ -5,6 +5,8 @@ QA output created by 143 | ||
| 144 | {"return": {}} | ||
| 145 | qemu-io: can't open device nbd+unix:///no_such_export?socket=SOCK_DIR/nbd: Requested export not available | ||
| 146 | server reported: export 'no_such_export' not present | ||
| 147 | +qemu-io: can't open device nbd+unix:///aa--aa1?socket=SOCK_DIR/nbd: Requested export not available | ||
| 148 | +server reported: export 'aa--aa...' not present | ||
| 149 | { 'execute': 'quit' } | ||
| 150 | {"return": {}} | ||
| 151 | {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} | ||
