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

[Kamil Rytarowski]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Ping!

This issue is still valid.

For example:

pulsecore/core-util.c: In function 'pa_atou':
  CC       pulsecore/libpulsecommon_6.0_la-flist.lo
pulsecore/core-util.c:2334:5: warning: array subscript has type 'char'
[-Wchar-subscripts]
     if (isspace(*s)) {
     ^
pulsecore/core-util.c: In function 'pa_atol':
pulsecore/core-util.c:2378:5: warning: array subscript has type 'char'
[-Wchar-subscripts]
     if (isspace(*s)) {
     ^
pulsecore/core-util.c: In function 'pa_atod':
pulsecore/core-util.c:2423:5: warning: array subscript has type 'char'
[-Wchar-subscripts]
     if (isspace(*s)) {
     ^


On 20.11.2015 04:20, Kamil Rytarowski 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) --- 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; }
> 
> diff --git a/src/pulse/proplist.c b/src/pulse/proplist.c index
> 4e2901b..d8c6404 100644 --- a/src/pulse/proplist.c +++
> b/src/pulse/proplist.c @@ -480,7 +480,7 @@ pa_proplist
> *pa_proplist_from_string(const char *s) { goto success; else if (*p
> == '=') goto fail; -                else if (!isspace(*p)) { +
> else if (!isspace((unsigned char)*p)) { key = p; state = KEY; 
> key_len = 1; @@ -492,7 +492,7 @@ pa_proplist
> *pa_proplist_from_string(const char *s) { goto fail; else if (*p ==
> '=') state = VALUE_START; -                else if (isspace(*p)) +
> else if (isspace((unsigned char)*p)) state = AFTER_KEY; else 
> key_len++; @@ -503,7 +503,7 @@ pa_proplist
> *pa_proplist_from_string(const char *s) { goto fail; else if (*p ==
> '=') state = VALUE_START; -                else if (!isspace(*p)) +
> else if (!isspace((unsigned char)*p)) goto fail; break;
> 
> @@ -523,7 +523,7 @@ pa_proplist *pa_proplist_from_string(const char
> *s) { state = VALUE_DOUBLE_QUOTES; value = p+1; value_len = 0; -
> } else if (!isspace(*p)) { +                } else if
> (!isspace((unsigned char)*p)) { state = VALUE_SIMPLE; value = p; 
> value_len = 1; @@ -531,7 +531,7 @@ pa_proplist
> *pa_proplist_from_string(const char *s) { break;
> 
> case VALUE_SIMPLE: -                if (*p == 0 || isspace(*p)) { +
> if (*p == 0 || isspace((unsigned char)*p)) { if (proplist_setn(pl,
> key, key_len, value, value_len) < 0) goto fail;
> 
> @@ -610,7 +610,7 @@ pa_proplist *pa_proplist_from_string(const char
> *s) { (*p >= 'A' && *p <= 'F') || (*p >= 'a' && *p <= 'f')) { 
> value_len++; -                } else if (*p == 0 || isspace(*p)) { 
> +                } else if (*p == 0 || isspace((unsigned char)*p))
> {
> 
> if (proplist_sethex(pl, key, key_len, value, value_len) < 0) goto
> fail; diff --git a/src/pulsecore/core-util.c
> b/src/pulsecore/core-util.c index 2099686..db93e86 100644 ---
> a/src/pulsecore/core-util.c +++ b/src/pulsecore/core-util.c @@
> -2329,7 +2329,7 @@ int pa_atou(const char *s, uint32_t *ret_u) { 
> pa_assert(ret_u);
> 
> /* strtoul() ignores leading spaces. We don't. */ -    if
> (isspace(*s)) { +    if (isspace((unsigned char)*s)) { errno =
> EINVAL; return -1; } @@ -2373,7 +2373,7 @@ int pa_atol(const char
> *s, long *ret_l) { pa_assert(ret_l);
> 
> /* strtol() ignores leading spaces. We don't. */ -    if
> (isspace(*s)) { +    if (isspace((unsigned char)*s)) { errno =
> EINVAL; return -1; } @@ -2418,7 +2418,7 @@ int pa_atod(const char
> *s, double *ret_d) { pa_assert(ret_d);
> 
> /* strtod() ignores leading spaces. We don't. */ -    if
> (isspace(*s)) { +    if (isspace((unsigned char)*s)) { errno =
> EINVAL; return -1; } diff --git a/src/pulsecore/ltdl-helper.c
> b/src/pulsecore/ltdl-helper.c index 4e003c9..cfdde26 100644 ---
> a/src/pulsecore/ltdl-helper.c +++ b/src/pulsecore/ltdl-helper.c @@
> -53,7 +53,7 @@ pa_void_func_t pa_load_sym(lt_dlhandle handle, const
> char *module, const char *s sn = pa_sprintf_malloc("%s_LTX_%s",
> module, symbol);
> 
> for (c = sn; *c; c++) -        if (!isalnum(*c)) +        if
> (!isalnum((unsigned char)*c)) *c = '_';
> 
> f = (pa_void_func_t) lt_dlsym(handle, sn); diff --git
> a/src/pulsecore/modargs.c b/src/pulsecore/modargs.c index
> 7b68346..b3c0313 100644 --- a/src/pulsecore/modargs.c +++
> b/src/pulsecore/modargs.c @@ -131,7 +131,7 @@ pa_modargs
> *pa_modargs_new(const char *args, const char* const* valid_keys) { 
> case WHITESPACE: if (*p == '=') goto fail; -                else if
> (!isspace(*p)) { +                else if (!isspace((unsigned
> char)*p)) { key = p; state = KEY; key_len = 1; @@ -141,7 +141,7 @@
> pa_modargs *pa_modargs_new(const char *args, const char* const*
> valid_keys) { case KEY: if (*p == '=') state = VALUE_START; -
> else if (isspace(*p)) +                else if (isspace((unsigned
> char)*p)) goto fail; else key_len++; @@ -156,7 +156,7 @@ pa_modargs
> *pa_modargs_new(const char *args, const char* const* valid_keys) { 
> state = VALUE_DOUBLE_QUOTES; value = p+1; value_len = 0; -
> } else if (isspace(*p)) { +                } else if
> (isspace((unsigned char)*p)) { if (add_key_value(ma, 
> pa_xstrndup(key, key_len), pa_xstrdup(""), @@ -175,7 +175,7 @@
> pa_modargs *pa_modargs_new(const char *args, const char* const*
> valid_keys) { break;
> 
> case VALUE_SIMPLE: -                if (isspace(*p)) { +
> if (isspace((unsigned char)*p)) { if (add_key_value(ma, 
> pa_xstrndup(key, key_len), pa_xstrndup(value, value_len),
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWY2BHAAoJEEuzCOmwLnZs/nMP/3rh9VTs9jbygHqEI+PeRebg
yxvb8cb46f3y0bvPo/BISNDu/I2y2qNw4dtisO+nJStmCP5mGeTb4ZOUdPzxJtNJ
CdMQaCLWftZe/QO0g7hjdAs77CBQo/VDh7K4G+QmgD0XjquQuVmW3RyHG1JPbkj5
lL4AiXMhZCg+29xa13nH9G9DEPCl7z70Oytx+ZPL5QDM/ZHEwoqbTbAoxmONmWk/
o/ZJl9ah+ssXpqi5AFeHv94ICwZjg1RNfZ9bV1bBmqiVkA4vWF0D818u0fmhQshP
wRun7mOi9ZQi7MKDuUi4WjgNzbLBRnQnkoX+GLMXzsamxRcOk3x13G/pR0YxZaCE
fe8qpVR2MyMiGPcCuoEqtuKSD2Hb8B6r1mVvX5VY+czMLoac51QjyMkfT+vy6Dcx
b4TifCuFuPOCsW42HcWk6h45AUItA0vFI7VerNzl19Wy+VsQ0Gq2cBzEQLStbQxh
1nBvAD9o7pKxEmHpEpwJRlu8jtCn1AMnhHUZNbDtU0tJgNCFnGeYVQMxmPH5u/6O
9sg7dtMb2UPbtg0il3ZkLzDvM22GgBTuMd2brPLknIWpxTsJ+kUEdeLEPA+W+Hgf
g7N5r76iDe6diy2S3brbrBF+Ejg/GkE5sWudNhB7p4b9SWyCtR9Q5aOuzrnZeF7i
vgw3I9Nb8F+wJG9nALwG
=WXHR
-----END PGP SIGNATURE-----