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

[Kamil Rytarowski]

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

On 08.12.2015 04:27, Arun Raghavan wrote:
> On 20 November 2015 at 15:52, Kamil Rytarowski <n54 at gmx.com>
> wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
>> 
>> On 20.11.2015 07:35, Arun Raghavan wrote:
>>> 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.
>>> 
>> 
>> This is a part of the C Standard. Behind the scenes ctype(3)
>> functions are table lookup functions, passing anything out of the
>> range results in undefined/invalid memory access.
>> 
>>>> --- 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.
>>> 
>> 
>> The isspace(3) function isn't designed to handle non-ASCII
>> characters. If we pass anything (except EOF) we always must cast
>> it, no matter what the program logic is.
>> 
>>> 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).
>>> 
>> 
>> This is separated task not handled by this commit.
> 
> Okay, that sounds fine. Pushing now.
> 
> -- Arun
> 
Thank you!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWZlEpAAoJEEuzCOmwLnZsgocP+QHrQ4dwKJ23mXoR03f6wPKB
nMp0XuaNzlA2st3eBjDkPmWLPBWMok9JTmZXYiuLNUt873nKCqatXYerhoia5kya
ZN9KoFKSALxryzXUK4uTOedttIOF/QKGlV7XAqdH+jvlSdh2Y/wYPfLguWHiBAoH
4VekVLYAIHE1rHFZFpZjoXhgfVnFPSEh0hIec2WI/1Vjn0mNCQrbKKKASWrFo8Q+
bv8/8fO1XK2K+o6favbWGXPFS1EBs/V0m3tyWT27ObJ1nvqC7GlKpF2h9a4UIDV5
h/BzBnuyWmBIGsHp2wwl6VV0A8QlLZ0OALjx8hqmUl3lke7U5c9pE86augv4+0BI
Pi2/Zrgf6dXn0MHix2ztjv0GRUJiMbH1H3o8BsQ8xzTfdS6VnXCMhzCptxJxnR2/
bbcEk9YKODYL9QsVltL/Ay5UX/O3K4A5Bz0WN5ec/mJq4+JGuGhwjwS+DIsPIZXK
DxqmBJm5bdWU+wodUdrL0+HYcTQ2VptqZjQ/5X/wOP1ZGAhnp2ho1i8l/1AvYiWI
j8KLJ7Tm4GO/f+eCC0tbI6kZ0sEGM3d5w9JMjyUtOn9LCr2DUtmpl/MItufRbaGu
0Qr9gSALv4He9V2UxHRuF8AMcufBCPWfae8OZCwgOEjx5Y3D3UpuQpEXQqwFOnrH
Qogk3LmHTcRj5HVprswM
=tJdS
-----END PGP SIGNATURE-----