Re: locale: Fix for too small buffers

[Vitaliy Margolen]

Yep that is the problem I was seeing with one program I'm running. And this
patch took care of it. Thanks!

Vitaliy Margolen

Tuesday, December 2, 2003, 13:01:18, Vitaliy Margolen wrote:
> Hi, found a problem with the latest locale changes. When it calls the 
> GetLocaleInfoW() function, the attached error occurs.
> This occurred because of the new code using the LOCALE_RETURN_NUMBER flag. The
> problem is if the buffer supplied to get_registry_locale_info is quite small 
> (say sizeof(INT)). The value returned by NtQueryValueKey() however, is for a 
> string, and is much longer. As NtQueryValueKey updates the value of size, 
> this caused other parts of the code to corrupt memory.

Re[2]: locale: Fix for too small buffers

[Vitaliy Margolen]

Found yet one more case with heap corruption:

=>0 0x401c2369 (HEAP_CreateFreeBlock+0x104(subheap=0x40300000, ptr=0x40364b78, size=0xb488) [heap.c:429] in NTDLL.DLL) (ebp=408dfc14)
  1 0x401c242a (HEAP_MakeInUseBlockFree+0x7d(subheap=0x40300000, pArena=0x40364b78) [heap.c:466] in NTDLL.DLL) (ebp=408dfc3c)
  2 0x401c3c30 (RtlFreeHeap+0x12a(heap=0x40300000, flags=0x2, ptr=0x40364b80) [heap.c:1202] in NTDLL.DLL) (ebp=408dfc68)
  3 0x4047d7bc (HeapFree+0x1e(heap=0x40300000, flags=0x0, ptr=0x40364b80) [heap.c:284] in KERNEL32.DLL) (ebp=408dfc80)
  4 0x4048bef9 (get_registry_locale_info+0x25e(flags=0x0, value=0x40512302, buffer=0x0, len=0x0) [locale.c:822] in KERNEL32.DLL) (ebp=408dfcc8)
  5 0x4048c15c (GetLocaleInfoW+0xec(lcid=0x419, lctype=0x28, buffer=0x0, len=0x0) [locale.c:933] in KERNEL32.DLL) (ebp=408dfd20)
  6 0x4048bf5b (GetLocaleInfoA+0x54(lcid=0x419, lctype=0x28, buffer=0x408dfd70, len=0x100) [locale.c:859] in KERNEL32.DLL) (ebp=408dfd50)

The heap corruption happens here:
  ((WCHAR *)info->Data)[ret] = '\0';

Should we move it somewhere else? Because in this case this is the only one
thing, that's being executed (it's not a number nor buffer is set).

BTW reading MSDN it's clearly stated, that:
"lpLCData
    [out] Pointer to a buffer that receives the requested data. This pointer is
    not used if cchData is zero."

Why are we using !buffer as an indication for this not the !len ?

I'm not sure if I want to submit a patch for this. There few things that I don't
feel comfortable about. Attached is something that fixed the problem for me. But
I have a gut feeling this function needs to be redone.

Friday, December 5, 2003, 9:29:34 AM, you wrote:
> Yep that is the problem I was seeing with one program I'm running. And this
> patch took care of it. Thanks!
> Vitaliy Margolen
> Tuesday, December 2, 2003, 13:01:18, Vitaliy Margolen wrote:
> > Hi, found a problem with the latest locale changes. When it calls the 
> > GetLocaleInfoW() function, the attached error occurs.
> > This occurred because of the new code using the LOCALE_RETURN_NUMBER flag. The
> > problem is if the buffer supplied to get_registry_locale_info is quite small 
> > (say sizeof(INT)). The value returned by NtQueryValueKey() however, is for a 
> > string, and is much longer. As NtQueryValueKey updates the value of size, 
> > this caused other parts of the code to corrupt memory.

locale: Fix for too small buffers

[Alexandre Julliard]

Vitaliy Margolen  writes:
> I'm not sure if I want to submit a patch for this. There few things that I don't
> feel comfortable about. Attached is something that fixed the problem for me. But
> I have a gut feeling this function needs to be redone.

Your feeling is correct, and in fact this function has already been
redone some days ago...

Re[2]: locale: Fix for too small buffers

[Vitaliy Margolen]

Doh, next time I'll do
#cvs update
before complaining that something doesn't work.

The only question I have is: should we use len==0 as indication for size query
or leave it as is (buffer==0)? I can't imagine someone using valid pointer when
calling GetLocaleInfo to get the size for it. But then who knows what other
programmers could do, considering what MSDN says.

Regards

Saturday, December 13, 2003, 1:18:28 PM, you wrote:
> Vitaliy Margolen  writes:
> > I'm not sure if I want to submit a patch for this. There few things that I don't
> > feel comfortable about. Attached is something that fixed the problem for me. But
> > I have a gut feeling this function needs to be redone.
> Your feeling is correct, and in fact this function has already been
> redone some days ago...

locale: Fix for too small buffers

[Alexandre Julliard]

Vitaliy Margolen  writes:
> The only question I have is: should we use len==0 as indication for size query
> or leave it as is (buffer==0)? I can't imagine someone using valid pointer when
> calling GetLocaleInfo to get the size for it. But then who knows what other
> programmers could do, considering what MSDN says.

GetLocaleInfoW takes care of that, it sets buffer to NULL if len is 0.