Re: ntdll: Fix RtlIntegerToUnicodeString so it won't overflow

[James Hawkins]

On Thu, May 8, 2008 at 1:00 PM, Maarten Lankhorst
 wrote:
> Hello Alexandre,
>
> 2008/5/8 Alexandre Julliard :
> > "Maarten Lankhorst"  writes:
> > > @@ -1970,7 +1970,7 @@ NTSTATUS WINAPI RtlIntegerToUnicodeString(
> > >      } while (value != 0L);
> > >  str->Length = (&buffer[32] - pos) * sizeof(WCHAR);
> > > - if (str->Length >= str->MaximumLength) {
> > > - if (str->Length + sizeof(WCHAR) >= str->MaximumLength) {
> > >  return STATUS_BUFFER_OVERFLOW;
> > > } else {
> > >  memcpy(str->Buffer, pos, str->Length + sizeof(WCHAR));
> >
> > There's no overflow here. The Windows implementation of
> >  RtlIntegerToUnicodeString seems badly confused but I don't think
> >  we need to replicate those bugs.
>
> It copies str->Length + sizeof(WCHAR) to the destination buffer
> according to james' testcases.

No, the length is indeterminate.
> So it definitely looks like a bugto me
> if it would copy data beyond MaximumLength, since only up to
> MaximumLength is guaranteed to be allocated. Of course you're right
> that my fix is likely wrong, the >= max should probablly be changed to
> > max, otherwise it would return STATUS_BUFFER_OVERFLOW wrongly.
>
> Cheers,
> Maarten.