Opened 4 years ago
Closed 4 years ago
#16397 closed defect (fixed)
Tor Browser crashes on some SVG images
Reported by: | mcap | Owned by: | mcs |
---|---|---|---|
Priority: | Very High | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Keywords: | tbb-crash, tbb-usability-website, TorBrowserTeam201506R | |
Cc: | mcs, brade, mikeperry, gk, arthuredelstein | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
Hello,
When I visit this link http://www.theplantlist.org/browse/B/Sphagnaceae/Sphagnum/ Tor Browser just closes even if I have multiple tabs/windows open.
Child Tickets
Change History (11)
comment:1 Changed 4 years ago by
Cc: | mcs brade added |
---|---|
Keywords: | tbb-crash tbb-usability-website added |
Priority: | normal → critical |
comment:2 Changed 4 years ago by
This crash is caused by a variation of the problem we fixed in #15794. The difference seems to be that the page at http://www.theplantlist.org/browse/B/Sphagnaceae/Sphagnum/ uses <object> to load the SVG instead of using an <svg> tag. Kathy and I are testing a fix.
comment:3 Changed 4 years ago by
Owner: | changed from tbb-team to mcs |
---|---|
Status: | new → assigned |
comment:4 Changed 4 years ago by
Keywords: | TorBrowserTeam201506R added |
---|---|
Status: | assigned → needs_review |
A fix is available here:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16397-01&id=2c1d0dada80bad43abba0f4a9d283bf5dc02c201
Please review.
This crash was in nsXMLContentSink.cpp (first change in our patch). But Kathy and I tried hard to locate each place where the Mozilla code assumed that a <script> element could be QI'd to an nsIScriptElement, and then we fixed the unsafe pointer dereferences.
I think this fix can wait for our next scheduled release. But it is a NULL pointer dereference and a potential denial of service for users who have the security slider set to the high setting.
comment:5 Changed 4 years ago by
Cc: | mikeperry gk added |
---|
comment:6 follow-up: 7 Changed 4 years ago by
Looks good to me (I have not tried to compile the patch and to run a respective build, though).
comment:7 Changed 4 years ago by
Cc: | arthuredelstein added |
---|
Replying to gk:
Looks good to me (I have not tried to compile the patch and to run a respective build, though).
Thanks for your review. It would be nice to include this patch in 5.0a3 so it gets some testing. Mike or Arthur, please provide a second review if you have time.
comment:9 follow-up: 10 Changed 4 years ago by
Status: | needs_review → needs_revision |
---|
This looks OK to me. I picked this branch up for the stable, but had to cheat and turn it into a patch. It seems to be based off of a weird branch..
It did not apply to the 38 branch at all, due to file relocations, etc. Can you make a new patch against tor-browser.git/tor-browser-38.1.0esr-5.x-1?
comment:10 Changed 4 years ago by
Status: | needs_revision → needs_review |
---|
Replying to mikeperry:
This looks OK to me. I picked this branch up for the stable, but had to cheat and turn it into a patch. It seems to be based off of a weird branch..
Hmmm. It should not have been based off anything weird. Thanks for fixing it.
It did not apply to the 38 branch at all, due to file relocations, etc. Can you make a new patch against tor-browser.git/tor-browser-38.1.0esr-5.x-1?
A rebased patch is available here:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16397-02&id=455a21820dff054fb8bde1417f3d0a289917d932
We updated to the latest tor-browser-38.1.0esr-5.x-1 a few hours ago and built and tested this, so it should be good to go.
comment:11 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Summary: | Tor Browser closes unexpectedly → Tor Browser crashes on some SVG images |
Ok, merged for 5.0a3 as well.
This happens on the high security slider setting and is due to the SVG patch. mcs, brade could you have a look? I tried to poke around with GDB a bit but it was just freezing my browser giving no good stack trace back. I might look at it later today again.