[pulseaudio-discuss] [PATCH] netbsd: Fix undefined behavior with array subscript of invalid type

[Arun Raghavan]

On 20 November 2015 at 08:50, Kamil Rytarowski <n54 at gmx.com> wrote:
> From the NetBSD manual:
>
>      The first argument of these functions is of type int, but only a very
>      restricted subset of values are actually valid.  The argument must either
>      be the value of the macro EOF (which has a negative value), or must be a
>      non-negative value within the range representable as unsigned char.
>      Passing invalid values leads to undefined behavior.
>
>      --  ctype(3)

This is also true for C99 in general, so not a NetBSD-specific thing, it seems.

> ---
>  src/modules/dbus/iface-core.c |  2 +-
>  src/pulse/proplist.c          | 12 ++++++------
>  src/pulsecore/core-util.c     |  6 +++---
>  src/pulsecore/ltdl-helper.c   |  2 +-
>  src/pulsecore/modargs.c       |  8 ++++----
>  5 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c
> index 1b14195..88e9030 100644
> --- a/src/modules/dbus/iface-core.c
> +++ b/src/modules/dbus/iface-core.c
> @@ -1442,7 +1442,7 @@ static bool contains_space(const char *string) {
>      pa_assert(string);
>
>      for (p = string; *p; ++p) {
> -        if (isspace(*p))
> +        if (isspace((unsigned char)*p))
>              return true;
>      }

I'm not sure how this is better -- we go from undefined behaviour in
the library to forcing potentially undefined behaviour ourselves --
non-ASCII values will generate a "random" value in ASCII space.

We should be checking for valid input instead if we care about this
(the one place I quickly checked that uses isspace() is preceded by a
pa_asci_valid() call).

-- Arun