[vlc-devel] [PATCH] fix null deref crash

[Alexandre Janniaux]

Hi,

On Thu, Sep 17, 2020 at 09:46:38PM +0100, Lyndon Brown wrote:
> On Thu, 2020-09-17 at 22:45 +0300, Rémi Denis-Courmont wrote:
> > Le torstaina 17. syyskuuta 2020, 22.34.37 EEST Lyndon Brown a écrit :
> > > a few plugins have recently introduced options with no shorttext,
> > > resulting in a null dereference crash when their respective entries
> > > are
> > > selected in the Qt advances prefs tree.
> >
> > Passing NULL for a translated string is not defined. The approach in
> > your patch
> > is not an adequate solution, because it cannot prevent the NULL
> > dereference
> > when NLS is disabled.
>
> Not defined? This concerns the VLC wrappers, not the actual gettext
> functions... Also, if it's UB, then there's no issue deviating, if
> indeed we are...
>
> With the fix in place, a NULL pointer passed into the wrapper is simply
> returned, which is the same as when ENABLE_NLS is not defined.
>
> I've tried the fix, and it works, thus the caller (Qt module code in
> this case) is clearly able to handle the NULL perfectly fine when
> returned for input to QString::fromUtf8(). I don't see any need to
> modify the Qt module (or others if they also cope fine) to handle a
> null in any special way.
>
> I'm afraid  don't see why the fix would be inadequate...
>
> > The NULs need to be replaced with valid strings.
>
> Granted, I'll submit the patches to address that. But things should
> robustly handle this situation rather than crash.

Not sure you can robustly manage to have the plugins fixed,
including future usage, while having a defensive approach.

Instead, enforcing with an assert might help spotting the
place where it's not respected.

Regards,
--
Alexandre Janniaux
Videolabs