5c4fe018c0
Ever since commit36683283(v2.8), the server code asserts that error strings sent to the client are well-formed per the protocol by not exceeding the maximum string length of 4096. At the time the server first started sending error messages, the assertion could not be triggered, because messages were completely under our control. However, over the years, we have added latent scenarios where a client could trigger the server to attempt an error message that would include the client's information if it passed other checks first: - requesting NBD_OPT_INFO/GO on an export name that is not present (commit0cfae925in v2.12 echoes the name) - requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is not present (commite7b1948din v2.12 echoes the name) At the time, those were still safe because we flagged names larger than 256 bytes with a different message; but that changed in commit93676c88(v4.2) when we raised the name limit to 4096 to match the NBD string limit. (That commit also failed to change the magic number 4096 in nbd_negotiate_send_rep_err to the just-introduced named constant.) So with that commit, long client names appended to server text can now trigger the assertion, and thus be used as a denial of service attack against a server. As a mitigating factor, if the server requires TLS, the client cannot trigger the problematic paths unless it first supplies TLS credentials, and such trusted clients are less likely to try to intentionally crash the server. We may later want to further sanitize the user-supplied strings we place into our error messages, such as scrubbing out control characters, but that is less important to the CVE fix, so it can be a later patch to the new nbd_sanitize_name. Consideration was given to changing the assertion in nbd_negotiate_send_rep_verr to instead merely log a server error and truncate the message, to avoid leaving a latent path that could trigger a future CVE DoS on any new error message. However, this merely complicates the code for something that is already (correctly) flagging coding errors, and now that we are aware of the long message pitfall, we are less likely to introduce such errors in the future, which would make such error handling dead code. Reported-by: Xueqiang Wei <xuwei@redhat.com> CC: qemu-stable@nongnu.org Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761 Fixes:93676c88d7Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20200610163741.3745251-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
76 lines
2 KiB
Bash
Executable file
76 lines
2 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
#
|
|
# Test case for connecting to a non-existing NBD export name
|
|
#
|
|
# Copyright (C) 2016 Red Hat, Inc.
|
|
#
|
|
# This program is free software; you can redistribute it and/or modify
|
|
# it under the terms of the GNU General Public License as published by
|
|
# the Free Software Foundation; either version 2 of the License, or
|
|
# (at your option) any later version.
|
|
#
|
|
# This program is distributed in the hope that it will be useful,
|
|
# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
# GNU General Public License for more details.
|
|
#
|
|
# You should have received a copy of the GNU General Public License
|
|
# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
#
|
|
|
|
# creator
|
|
owner=mreitz@redhat.com
|
|
|
|
seq="$(basename $0)"
|
|
echo "QA output created by $seq"
|
|
|
|
status=1 # failure is the default!
|
|
|
|
_cleanup()
|
|
{
|
|
_cleanup_qemu
|
|
rm -f "$SOCK_DIR/nbd"
|
|
}
|
|
trap "_cleanup; exit \$status" 0 1 2 3 15
|
|
|
|
# get standard environment, filters and checks
|
|
. ./common.rc
|
|
. ./common.filter
|
|
. ./common.qemu
|
|
|
|
_supported_fmt generic
|
|
_supported_proto generic
|
|
|
|
keep_stderr=y \
|
|
_launch_qemu 2> >(_filter_nbd)
|
|
|
|
_send_qemu_cmd $QEMU_HANDLE \
|
|
"{ 'execute': 'qmp_capabilities' }" \
|
|
'return'
|
|
|
|
_send_qemu_cmd $QEMU_HANDLE \
|
|
"{ 'execute': 'nbd-server-start',
|
|
'arguments': { 'addr': { 'type': 'unix',
|
|
'data': { 'path': '$SOCK_DIR/nbd' }}}}" \
|
|
'return'
|
|
|
|
# This should just result in a client error, not in the server crashing
|
|
$QEMU_IO_PROG -f raw -c quit \
|
|
"nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \
|
|
| _filter_qemu_io | _filter_nbd
|
|
# Likewise, with longest possible name permitted in NBD protocol
|
|
$QEMU_IO_PROG -f raw -c quit \
|
|
"nbd+unix:///$(printf %4096d 1 | tr ' ' a)?socket=$SOCK_DIR/nbd" 2>&1 \
|
|
| _filter_qemu_io | _filter_nbd | sed 's/aaaa*aa/aa--aa/'
|
|
|
|
_send_qemu_cmd $QEMU_HANDLE \
|
|
"{ 'execute': 'quit' }" \
|
|
'return'
|
|
|
|
wait=1 _cleanup_qemu
|
|
|
|
# success, all done
|
|
echo '*** done'
|
|
rm -f $seq.full
|
|
status=0
|