Re: [PATCH v1.1] __gconv_translit_find: Actually append ".so" to module name [BZ #17187]

["Carlos O'Donell" <carlos at redhat dot com>]

On 08/26/2014 12:55 PM, Florian Weimer wrote:
> On 08/26/2014 06:45 PM, Carlos O'Donell wrote:
>> On 08/21/2014 01:33 PM, Florian Weimer wrote:
>>> On 07/21/2014 03:01 PM, Florian Weimer wrote:
>>>> The previous code wrote the string after the terminating NUL character
>>>> of the existing module name.  This had two effects: the ".so" extension
>>>> was not actually visible in the module name string, and a NUL byte was
>>>> written byond the end of the allocated buffer.
>>>>
>>>> I'm not sure how to add a functionality test for this.  The test suite
>>>> does not show any changes in behavior.
>>>
>>> The attached version of this patch is suitable for backporting and
>>> disables external module loading, rather than attempting to fix it.
>>> The tests for the internal transliteration functionality still pass.
>>>
>>> I would like to suggest to put this in master, and include the
>>> second, larger patch as a clean-up after the 2.20 release.
>>
>> My assumption here is that we are 100% certain nobody could work around
>> the interface bugs (name bug, and failure to copy trans bug) and make
>> use of this functionality.
> 
> I think it is impossible to work around these bugs using public
> interfaces only.
> 
> In addition, I searched for the names of the ELF symbols which must
> be provided in by any transliteration module, and neither on the
> public web, nor in ELF symbols from shipped RPM files, I could find
> even remotely relevant hits.

Right, so as we suspected this is an unused feature. Which is good.
 
>> OK to commit from my perspective with one nit i.e. NEWS needs expanding.
> 
> Thanks.
> 
>> Suggest:
>>
>> * Support for loadable gconv transliteration modules has been removed.
>>    The support for transliteration modules has been non-functional for
>>    over a decade, and the removal is prompted by security defects.  The
>>    normal gconv conversion modules are still supported.  Transliteration
>>    is still possible, but only to and from installed languages.
>>    (CVE-2014-5519)
>>
>> I want to be clear that (a) for security and functionality reasons we
>> removed the code (b) normal gconv modules work and (c) transliteration
>> still works.
> 
> We do not support transliteration for the source character set, per this code in iconv/gconv_open.c:
> 
>   /* For the source character set we ignore the error handler specification.
>      XXX Is this really always the best?  */
>   ignore = strchr (fromset, '/');
>   if (ignore != NULL && (ignore = strchr (ignore + 1, '/')) != NULL
>       && *++ignore != '\0')
>     {
>       char *newfromset = (char *) alloca (ignore - fromset + 1);
> 
>       newfromset[ignore - fromset] = '\0';
>       fromset = memcpy (newfromset, fromset, ignore - fromset);
>     }
> 
> The internal transliteration code only supports TRANSLIT and IGNORE, so I suggest this NEWS entry:
> 
> * Support for loadable gconv transliteration modules has been removed.
>   The support for transliteration modules has been non-functional for
>   over a decade, and the removal is prompted by security defects.  The
>   normal gconv conversion modules are still supported.  Transliteration
>   with //TRANSLIT is still possible, and the //IGNORE specifier
>   continues to be  supported. (CVE-2014-5519)

OK, that's a more accurate description.

OK with that.

Cheers,
Carlos.