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

[Peter Meerwald-Stadler]

> > 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)
 
> > 
> >                     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