[pulseaudio-discuss] [PATCH v2 10/10] raop: Fix potential NULL dereference

[Hajime Fujita]

> On Mar 8, 2017, at 9:11 AM, Peter Meerwald-Stadler <pmeerw at pmeerw.net> wrote:
> 
> 
>>> diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
>>> index d329a09..5248691 100644
>>> --- a/src/modules/raop/raop-client.c
>>> +++ b/src/modules/raop/raop-client.c
>>> @@ -1254,13 +1254,13 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp, pa_rtsp_state_t state, pa_rtsp_st
>>>                    pa_xfree(token);
>>>                }
>>> 
>>> -                if (pa_safe_streq(mth, "Basic")) {
>>> +                if (pa_safe_streq(mth, "Basic") && realm) {
>>>                    rtrim_char(realm, '\"’);
>> 
>> I would remove `rtrim_char(realm, '\"’);` from this block and keep the if condition as-is, since realm is not used later.
> 
> even though realm is not referenced in the code, RFC2617 requires it to be 
> present; so I think we should check for it (before we assumed it is 
> present)

Okay, then it makes sense.

> 
>>> 
>>>                    pa_raop_basic_response(DEFAULT_USER_NAME, c->password, &response);
>>>                    ath = pa_sprintf_malloc("Basic %s",
>>>                        response);
>>> -                } else if (pa_safe_streq(mth, "Digest")) {
>>> +                } else if (pa_safe_streq(mth, "Digest") && realm && nonce) {
>> 
>> Why don’t we do like this:
>> +                    if (realm == NULL) {
>> +                        pa_log_error("realm not provided");
>> +                        goto error;
>> +                    } else if (nonce == NULL) {
>> +                        pa_log_error("nonce not provided");
>> +                        goto error;
>> +                    }
> 
> ok, this adds more code; my goal was addressing the NULL dereference in 
> case realm and/or nonce was missing
> 
> p.
> 
> -- 
> 
> Peter Meerwald-Stadler
> Mobile: +43 664 24 44 418