[MPlayer-dev-eng] [PATCH] fix SAMI parsing

[Howard Chu]

Reimar Döffinger wrote:
> On Sun, Jun 06, 2010 at 07:24:46PM -0400, hyc at highlandsun.com wrote:
>> On Mon, Jun 07, 2010 at 12:26:33AM +0200, Reimar D?ffinger wrote:
>>> On Sun, Jun 06, 2010 at 02:51:29PM -0700, Howard Chu wrote:
>>>
>>>> +		uint32_t c = strtol(s+2,&s, 0);
>>>> +		uint8_t tmp;
>>>> +		PUT_UTF8(c, tmp, *p++ = tmp;)
>>>> +		if (*s == ';') s++; }
>>>
>>> I'm sorry, but I think you'll have to somehow "prove"
>>> this is not a security issue.
>>> Checking and documenting that PUT_UTF8 will never write
>>> more than we read might be possible.
>>> Or just "blindly" checking we still have at least 8 bytes
>>> free should do as well.
>>
>> Not necessary. Decimal numbers encode only 3.25 bits per byte, while UTF-8 encodes 7 bits per byte. This conversion will always fit.
>
> Given that 3.25 bits just don't exist, it obviously is not that easy.
> And you forgot another case as well: negative numbers.
> This kind of thing just can't be done hand-wavy like that, or you'll
> always miss a case.

Fair enough. We can borrow from libldap/utf-8.c:

#define LDAP_UCS_UTF8LEN(c) \
     (c < 0x80 ? 1 : (c < 0x800 ? 2 : (c < 0x10000 ? 3 : \
     (c < 0x200000 ? 4 : (c < 0x4000000 ? 5 : 6)))))

and check the remaining buffer length.

Of course, we could just check for a '-' in the string and drop this entity. 
Nobody should be using negative character codes.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/