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

[Florian Weimer <fweimer at redhat dot com>]

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.

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

--
Florian Weimer / Red Hat Product Security