• All submissions to this site are governed by Second Life Project Contribution Agreement. By submitting patches and other information using this site, you acknowledge that you have read, understood, and agreed to those terms.
Issue Details (XML | Word | Printable)

Key: VWR-5430
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Normal Normal
Assignee: James Linden
Reporter: Angus Boyd
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
1. Second Life Viewer - VWR

Stack Corruption in "LLMIMETypes::parseMIMETypes"

Created: 08/Mar/08 08:32 AM   Updated: 05/Aug/08 04:30 PM
Component/s: Crashes
Affects Version/s: 1.19.1 Release Candidate
Fix Version/s: 1.19.1.4

Source Version: http://secondlife.com/developers/opensource/downloads/2008/03/slviewer-src-Branch_1-19-1-Viewer-r81609.zip
Linden Lab Issue ID: DEV-11803
Patch attached: Patch attached


 Description  « Hide
There are two stack corruption errors in function "LLMIMETypes::parseMIMETypes" due to improper pointer casting. Both of them were detected automatically by the "Basic Runtime Checks" compiler option of Visual C++ 7.1.

It is wrong to cast a pointer to the C++ type 'bool' to a pointer to the 'BOOL' type (which is just an integer) and perform a read/write access. The C++ 'bool' type is usually just one byte in size while the 'BOOL' type usually needs 4 bytes. The called function will therefore write its result into a 'BOOL' type (actually an integer with 4 bytes) while the storage which was allocated by the compiler is though only 1 byte in size. Thus, the stack gets corrupted near the 'LLMIMEWidgetSet info' variable.

I admit that I do not have an answer why this does not crash in the official RC build yet. It depends of course of stack layout, compiler alignment options, etc. whether this stack corruption occurs eventually. But, please be aware, that even if that bug does not yet cause any crash in the official released RC, it is nevertheless a time bomb which will sooner or later lead to a crash even in your builds.

Regards

The fix for this issue could be applied as shown below

bool LLMIMETypes::parseMIMETypes(const LLString& xml_filename)

{ ... ... ... }

else if (node->hasName("widgetset"))
{
...
...
...
if (child->hasName("allow_resize"))

{ BOOL bValue; child->getBoolValue( 1, &bValue ); info.mAllowResize = (bool)bValue; }

if (child->hasName("allow_looping"))

{ BOOL bValue; child->getBoolValue( 1, &bValue ); info.mAllowLooping = (bool)bValue; }

...
...
...



 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Maxfox Rau added a comment - 11/Mar/08 06:14 AM
It doesn't crash because there is no stack curruption in that case.. the stack is always long word aligned, so the 'bool' datatype is 4 bytes long on the stack, the same as a BOOL.

Angus Boyd added a comment - 11/Mar/08 09:24 AM
The 2 bool members of "LLMIMEWidgetSet" are both only 1 byte in size and they are both allocated on byte boundaries (not on integer boundaries). This is even the expected behavior of VC++ and it is documented as well.

In fact, a write access to the 1st bool member ("mAllowResize") will also overwrite the 2nd bool member ("mAllowLooping") and it will also overwrite 2 bytes on the stack which are following the "LLMIMEWidgetSet" structure. Whether this leads to a noticeable stack corruption with data loss, malfunction or a crash depends on several other things, but it is nevertheless a stack corruption - not to mention the wrong r/w access to the bool members which will give 'random' results due to accessing 3 uninitialized bytes while there should be accessed only 1.

However, just check the address of the 1st and the 2nd bool member of "LLMIMEWidgetSet" carefully and you will see what I mean.

Regards


Tofu Linden added a comment - 11/Mar/08 02:52 PM
Thanks for noticing - this was fixed internally a few days ago.

James Linden added a comment - 11/Mar/08 09:02 PM
Good catch. Yes, this is real stack corruption. I believe it is fixed in 1.19.1 RC1.