[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 1/5] qemu-bridge-helper: Fix misuse of isspace()
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PULL 1/5] qemu-bridge-helper: Fix misuse of isspace() |
Date: |
Thu, 30 May 2019 12:06:30 +0100 |
On Wed, 22 May 2019 at 14:49, Markus Armbruster <address@hidden> wrote:
>
> parse_acl_file() passes char values to isspace(). Undefined behavior
> when the value is negative. Not a security issue, because the
> characters come from trusted $prefix/etc/qemu/bridge.conf and the
> files it includes.
>
> Furthermore, isspace()'s locale-dependence means qemu-bridge-helper
> uses the user's locale for parsing $prefix/etc/bridge.conf. Feels
> wrong.
>
> Use g_ascii_isspace() instead. This fixes the undefined behavior, and
> makes parsing of $prefix/etc/bridge.conf locale-independent.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> Message-Id: <address@hidden>
> ---
> qemu-bridge-helper.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Coverity complains about this change (CID 1401706) because
it doesn't have enough information to know that the table
lookup g_ascii_isspace does is always safe:
tainted_data: Using tainted variable
(guchar)*(argend - 1) as an index to pointer g_ascii_table.
We know this is OK because we know the table is big enough that
a guchar index can't possibly overrun, but because the table
is declared in the glib header file as
GLIB_VAR const guint16 * const g_ascii_table;
Coverity has no idea of its size and is being pessimistic.
I've squashed the Coverity issue as a false-positive, but I
mention it here in case you thought it worth trying to write
something in coverity-model.c to provide a better model of
the glib function.
thanks
-- PMM
- [Qemu-devel] [PULL 0/5] Miscellaneous patches for 2019-05-22, Markus Armbruster, 2019/05/22
- [Qemu-devel] [PULL 1/5] qemu-bridge-helper: Fix misuse of isspace(), Markus Armbruster, 2019/05/22
- Re: [Qemu-devel] [PULL 1/5] qemu-bridge-helper: Fix misuse of isspace(),
Peter Maydell <=
- [Qemu-devel] [PULL 5/5] cutils: Simplify how parse_uint() checks for whitespace, Markus Armbruster, 2019/05/22
- [Qemu-devel] [PULL 3/5] gdbstub: Reject invalid RLE repeat counts, Markus Armbruster, 2019/05/22
- [Qemu-devel] [PULL 4/5] gdbstub: Fix misuse of isxdigit(), Markus Armbruster, 2019/05/22
- [Qemu-devel] [PULL 2/5] tests/vhost-user-bridge: Fix misuse of isdigit(), Markus Armbruster, 2019/05/22
- Re: [Qemu-devel] [PULL 0/5] Miscellaneous patches for 2019-05-22, Peter Maydell, 2019/05/23