527207 - NPE in XMLObject::XMLObject - missing guard(s)

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Lars T Hansen]

Attaching message from Dave Horton below.  Could look like the XMLObject::XMLObject code has several unguarded uses of a possibly-NULL 'p' value, depending a little on how the XML parser is supposed to work.  There aren't any obvious preconditions on when this constructor is called.

----

On 11/6/09 2:37 AM, "Dave Horton" <[e-mail]> wrote:

Slight clarification...it actually segfaults a couple lines below 
where I indicated, where it tries to call a method on the null pointer 
p..the last line below:
                if ( p != m_node && ! m_status )
                {
                        Multiname m;
                        p->getQName(core, &m);
On Nov 5, 2009, at 8:20 PM, Dave Horton wrote:

> I've noticed what seems to be a bug in the version of the 
> constructor for XMLObject that takes a Stringp:
>
>       // This is considered the "toXML function"
>       XMLObject::XMLObject(XMLClass *type, Stringp str, Namespace 
> *defaultNamespace)
>               : ScriptObject(type->ivtable(), type->prototype)
>
> If I pass a string containing only a single "empty" element then it 
> seg faults on this line near the bottom of the method:
>
>               if ( p != m_node && ! m_status )
>
>
> For instance, if I feed it the string... <child name="Joe"/>
>
> it will bomb out, because in the parser a single tag is returned, 
> which is empty, and therefore the local var E4XNode* p is never 
> updated.
>
> Am I misunderstanding how this function can be used?  The example 
> xml snippet above is valid XML and seems like it should be able to 
> be parsed....

Comment 1

[Dan Smith]

Werner, can you investigate this issue?

Comment 2

[Werner Sharp]

There are two places in Tamarin/Flash that we call this XMLObject constructor and in both places we pass in a defaultNamespace value.  If we don't pass in a value and a simple XML string, we would get into this code with a null "p" value.  The simple fix would just be a check for a null 'p':

		if ( p && p != m_node && ! m_status )

This is not important for the 10.1 release.

Comment 3

[Werner Sharp]

Attachment: null ptr check

Comment 4

[Dan Smith]

Assigning to Steven for landing.

Comment 5

[Dan Smith]

Ruchi, land this in TR in i9.

Comment 6

[Ruchi Lohani]

This has already landed in TR as 4759:5c7c8ae01f34 by Werner.