Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15794 closed defect (fixed)

crash on some SVG pages when svg.in-content.enabled=false

Reported by: mcs Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-4.5-alpha, TorBrowserTeam201504R
Cc: brade, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Kathy and I found a web page that consistently causes a NULL pointer dereference during page load in TB 4.5 when svg.in-content.enabled is set to false (e.g., when the security slider is set to the highest position).

We are working on a fix; the problem is related to the fact that elements with the SVG XML namespace are created as regular XML elements when svg.in-content.enabled=false.

Child Tickets

Change History (5)

comment:1 Changed 4 years ago by gk

Cc: mikeperry added
Keywords: tbb-4.5-alpha added
Priority: majorcritical

comment:2 Changed 4 years ago by mcs

Keywords: TorBrowserTeam201504R added
Status: newneeds_review

comment:3 Changed 4 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

Ok, this looks good as a fix. I merged it and Georg and I started a rebuild.

However, this issue makes me a bit more nervous about the namespace-based solution in general. Can we be reasonably sure that there are no other potential issues with this namespace change either allowing scripts to execute when they shouldn't, or allowing strange codepaths?

If not, we may want to create a pile of test cases for this type of stuff when we do the ff38-esr rebase and patch submission to Mozilla.

comment:4 in reply to:  3 ; Changed 4 years ago by mcs

Replying to mikeperry:

Ok, this looks good as a fix. I merged it and Georg and I started a rebuild.

However, this issue makes me a bit more nervous about the namespace-based solution in general. Can we be reasonably sure that there are no other potential issues with this namespace change either allowing scripts to execute when they shouldn't, or allowing strange codepaths?

Thanks for reviewing the fix. I am reasonably confident that scripts will not execute, etc. but the codepaths are complicated enough that it is difficult to be 100% certain. This specific problem occurred because Kathy and I did not consider the fact that the HTML parser would do special things with some elements such as <style> and <script> when enclosed within <svg> blocks. In fact, it does not seem to be doing too much, e.g., just recording line numbers (presumably so that it can report the correct one in the inspector tool). And the crash occurred because when SVG is disabled the elements are not created the same way (which is good, because that prevents special behavior when the pages are rendered but bad because we had not looked at this codepath).

If not, we may want to create a pile of test cases for this type of stuff when we do the ff38-esr rebase and patch submission to Mozilla.

I agree and I opened #15802 to track adding test cases. I will also take another look at <script> right now and test to ensure that JS is not executed when svg.in-content.enabled=false

comment:5 in reply to:  4 Changed 4 years ago by mcs

Replying to mcs:

I will also take another look at <script> right now and test to ensure that JS is not executed when svg.in-content.enabled=false

Confirmed through manual tests.

Note: See TracTickets for help on using tickets.