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

[Marvin Scholz]

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.

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