[vlc-devel] [PATCH] contrib: ass: Add patch to fix crash on macOS

[Steve Lhomme]

On 2019-11-07 10:11, Marvin Scholz wrote:
> On 7 Nov 2019, at 7:35, Steve Lhomme wrote:
> 
>> To be more consistent with the existing code you could return "" 
>> instead of NULL.
> 
> The return value is not a const string so returning „“ would require 
> allocating a
> 1-byte heap to store the \0, no? And then the caller would still need to 
> handle
> NULL return, as then both the malloc in the error case and the malloc in 
> cfstr2buf
> could fail.

In fact there's a (SAFE) CFRelease() further down, so it wouldn't work.

LGTM

>>
>> On 2019-11-07 0:06, Marvin Scholz wrote:
>>> Fixes a segfault in libass get_font_file function due to
>>> insufficient checks of return values.
>>> ---
>>>   contrib/src/ass/coretext-errorhandling.patch | 27 ++++++++++++++++++++
>>>   contrib/src/ass/rules.mak                    |  1 +
>>>   2 files changed, 28 insertions(+)
>>>   create mode 100644 contrib/src/ass/coretext-errorhandling.patch
>>>
>>> diff --git a/contrib/src/ass/coretext-errorhandling.patch 
>>> b/contrib/src/ass/coretext-errorhandling.patch
>>> new file mode 100644
>>> index 0000000000..29401e48dc
>>> --- /dev/null
>>> +++ b/contrib/src/ass/coretext-errorhandling.patch
>>> @@ -0,0 +1,27 @@
>>> +diff --git a/libass/ass_coretext.c b/libass/ass_coretext.c
>>> +index 59a8a2d..7371f7c 100644
>>> +--- a/libass/ass_coretext.c
>>> ++++ b/libass/ass_coretext.c
>>> +@@ -96,7 +96,13 @@ static bool check_glyph(void *priv, uint32_t code)
>>> + static char *get_font_file(CTFontDescriptorRef fontd)
>>> + {
>>> +     CFURLRef url = CTFontDescriptorCopyAttribute(fontd, 
>>> kCTFontURLAttribute);
>>> ++    if (!url)
>>> ++        return NULL;
>>> +     CFStringRef path = CFURLCopyFileSystemPath(url, 
>>> kCFURLPOSIXPathStyle);
>>> ++    if (!path) {
>>> ++        SAFE_CFRelease(url);
>>> ++        return NULL;
>>> ++    }
>>> +     char *buffer = cfstr2buf(path);
>>> +     SAFE_CFRelease(path);
>>> +     SAFE_CFRelease(url);
>>> +@@ -133,7 +139,7 @@ static void process_descriptors(ASS_Library 
>>> *lib, ASS_FontProvider *provider,
>>> +         int index = -1;
>>> +
>>> +         char *path = get_font_file(fontd);
>>> +-        if (strcmp("", path) == 0) {
>>> ++        if (!path || strcmp("", path) == 0) {
>>> +             // skip the font if the URL field in the font 
>>> descriptor is empty
>>> +             free(path);
>>> +             continue;
>>> diff --git a/contrib/src/ass/rules.mak b/contrib/src/ass/rules.mak
>>> index 1849987447..364afbad17 100644
>>> --- a/contrib/src/ass/rules.mak
>>> +++ b/contrib/src/ass/rules.mak
>>> @@ -43,6 +43,7 @@ $(TARBALLS)/libass-$(ASS_VERSION).tar.gz:
>>>   libass: libass-$(ASS_VERSION).tar.gz .sum-ass
>>>       $(UNPACK)
>>>       $(APPLY) $(SRC)/ass/ass-macosx.patch
>>> +    $(APPLY) $(SRC)/ass/coretext-errorhandling.patch
>>>   ifdef HAVE_WIN32
>>>       $(APPLY) $(SRC)/ass/use-topendir.patch
>>>       $(APPLY) $(SRC)/ass/libass-no-tchar.patch
>>> -- 
>>> 2.20.1 (Apple Git-117)
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel