Re: [Qemu-devel] [PULL 1/5] qemu-bridge-helper: Fix misuse of isspace()

[Peter Maydell]

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