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

RESOLVED FIXED in Q1 12 - Brannan

Status

P2
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: lhansen, Assigned: rulohani)

Tracking

unspecified
Q1 12 - Brannan
x86
Mac OS X
Bug Flags:
flashplayer-injection -
flashplayer-qrb +
flashplayer-bug +
flashplayer-needsversioning -

Details

(Whiteboard: has-patch, loose-end)

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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" <beachdog@me.com> 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

9 years ago
Werner, can you investigate this issue?
Assignee: nobody → wsharp
Status: NEW → ASSIGNED
Flags: flashplayer-qrb+
Priority: -- → P2
Target Milestone: --- → flash10.1

Comment 2

9 years ago
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.

Updated

9 years ago
Target Milestone: flash10.1 → flash10.2

Comment 3

9 years ago
Created attachment 448567 [details] [diff] [review]
null ptr check
Attachment #448567 - Flags: superreview?(edwsmith)
Attachment #448567 - Flags: review?(stejohns)

Updated

9 years ago
Attachment #448567 - Flags: review?(stejohns) → review+

Updated

9 years ago
Attachment #448567 - Flags: superreview?(edwsmith) → superreview+

Updated

8 years ago
Assignee: wsharp → nobody

Updated

8 years ago
Flags: flashplayer-bug+
Whiteboard: has-patch, must–fix-candidate

Updated

8 years ago
Whiteboard: has-patch, must–fix-candidate → has-patch, must-fix-candidate

Comment 4

8 years ago
Assigning to Steven for landing.
Assignee: nobody → stejohns
Flags: flashplayer-injection-

Updated

8 years ago
Target Milestone: Q3 11 - Serrano → Q4 11 - Anza

Comment 5

7 years ago
Ruchi, land this in TR in i9.
Assignee: stejohns → rulohani
Flags: flashplayer-needsversioning-
Whiteboard: has-patch, must-fix-candidate → has-patch, loose-end
Target Milestone: Q4 11 - Anza → Q1 12 - Brannan
(Assignee)

Comment 6

7 years ago
This has already landed in TR as 4759:5c7c8ae01f34 by Werner.
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.