Re: stringprep() doesn't match documentation

[Thijs Alkemade]

On 25 nov. 2014, at 23:18, Simon Josefsson <address@hidden> wrote:

> Thijs Alkemade <address@hidden> writes:
> 
>> Hi,
>> 
>> The documentation for the stringprep() function states:
>> 
>> "[I]ndicate how large the buffer holding the string is. This function will 
>> not
>> _read_ or write to characters outside that size."
>> 
>> Looking at the source, this is clearly not true: the maxlen argument to
>> stringprep() is only used at the end to check whether the resulting string
>> fits in the buffer. The stringprep_utf8_to_ucs4() call is not restricted to
>> stay within any limits.
>> 
>> This means that, if an attacker is able to inject invalid UTF-8 into the 
>> input
>> buffer used for stringprep(), the lack of error checking by
>> stringprep_utf8_to_ucs4() can be used to skip over the actual terminating
>> NULL-byte, causing he stringprep call to read memory past the buffer it was
>> supposed to not read outside of. Sure, this is the application's fault for 
>> not
>> properly veryfing the input is UTF-8, but the mismatch between the
>> documentation and the function makes this worse.
> 
> If the input string is valid UTF-8, I believe there is no problem.  Do
> you agree?

I agree, as long as applications abide by the API, there is no problem.

> Applications should not pass unvalidated strings to stringprep(), it
> must be checked to be valid UTF-8 first.  If stringprep() receives
> non-UTF8 inputs, I believe there are other similar serious things that
> can happen.
> 
> Quoting the docstring:
> 
> * Prepare the input zero terminated UTF-8 string according to the
> * stringprep profile, and write back the result to the input string.
> 
> Admittedly, the library could 1) provide functions for checking whether
> a string is valid UTF-8, and/or 2) actually validate that inputs are
> UTF-8 before using them.  The latter may cause a small performance
> penalty, but probably not that much.  Further thinking or suggestions in
> this direction is welcome.

A minimal fix would be to ensure that the function never reads past maxlen,
whatever the input is. From the documentation of stringprep_utf8_to_ucs4(),
passing maxlen as the second parameter at stringprep.c:382 should be enough.

When I requested a CVE id for a vulnerability in jabberd2 that used this
behaviour of stringprep(), they decided to assign a separate CVE id to libidn,
CVE-2015-2059:

http://seclists.org/oss-sec/2015/q1/672

Best regards,
Thijs Alkemade

signature.asc
Description: Message signed with OpenPGP using GPGMail