mmio: Ensure FOURCC string conversions are null-terminated.

Eric Pouech eric.pouech at orange.fr
Sun Jul 18 10:53:20 CDT 2010


the explanations for the bug fix look really strange
all the printf use %4.4s form, which will limit output to only 4 chars,
whatever the string is terminated or not
so the crash doesn't come from here
looking quickly at the patch, several remark arise
- in the second & third modification to the TRACE call, are you sure the
test to srcType is relevant (I don't have the code handy, but it may be a
wrong cut & paste)
- more importantly, as the %4.4s form implies a 4 char strings, the strings
used in case of NULL value must be 4 characters long, instead we may get
into trouble
A+
2010/7/18 Tim Cadogan-Cowper <tccowper at yahoo.com.au>

> Three TRACE debug calls in the mmioDescend function of mmio.c attempt
> to print FOURCC data, by directly casting it to LPCSTR.  This is
> dangerous as: (i) FOURCC data types are DWORDs, i.e. four bytes (one for
> each char), with no null terminating byte (see
> http://msdn.microsoft.com/en-us/library/dd375802%28VS.85%29.aspx); and
> (ii) it assumes the byte order on the host architecture is little  endian.
>
> Because the LPCSTR casts are not necessarily null-terminated there will
> occasionally be buffer overflows in mmio's TRACE debug strings, with a
> consequent crash to desktop and debug output beginning with
>  'wine_dbg_vprintf:
> debugstr buffer overflow...'  For a consistent and predictable case  where
> this
> occurs, see Bug 10280 "Oblivion: Horse Armour Crash"
> (http://bugs.winehq.org/show_bug.cgi?id=10280).
>
> This patch fixes the problem (and any crashes caused by it) by using a
> new internal function in mmio - mmioFOURCCToString - which converts  FOURCC
> data to strings that are correctly null-terminated and respect byte  order.
> I have tested with Oblivion and can confirm it fixes Bug 10280 on my
> machine (ubuntu 10.04).  Furthermore the patch will ensure that chunk  IDs
> and
> fourccTypes are correctly printed in the mmio debug channel (at the  moment
> they are not).
>
> Tim
>
>
>
>
>
>


-- 
-- 
Eric Pouech
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-patches/attachments/20100718/6d9633a6/attachment.htm>


More information about the wine-patches mailing list