906601 - Improper tag parsing causing crash

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[marekzmyslowski]

Attachment: Firefox Crash.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130730113002

Steps to reproduce:

Run the attached html


Actual results:

Tags that are not improper close can cause stack overflow

Comment 1

[Loic]

This testcase doesn't crash FF23 or FF26 on my side. COuld you post some crash reports from about:support (bp-...), please.

Comment 2

[marekzmyslowski]

I have updated FF to 23.0.1. Below you can find the WinDBG dump

------------------------------------------------------------------------
b84.4e0): Break instruction exception - code 80000003 (first chance)
ntdll!DbgBreakPoint:
00000000`77310530 cc              int     3
0:027> g
1376909102937	Services.HealthReport.HealthReporter	WARN	No prefs data found.

ModLoad: 6d200000 6d22e000   C:\Windows\SysWOW64\shdocvw.dll
ModLoad: 6f690000 6f78b000   C:\Windows\SysWOW64\WindowsCodecs.dll
ModLoad: 6c850000 6c881000   EhStorAPI.DLL
ModLoad: 00000000`6bfe0000 00000000`6c011000   C:\Windows\SysWOW64\EhStorShell.dll
ModLoad: 00000000`66a10000 00000000`66e1a000   GrooveEX.DLL
ModLoad: 00000000`664f0000 00000000`668fa000   C:\PROGRA~2\MICROS~4\Office14\GROOVEEX.DLL
ModLoad: 00000000`74a00000 00000000`74aa3000   C:\Windows\WinSxS\x86_microsoft.vc90.crt_1fc8b3b9a1e18e3b_9.0.30729.6161_none_50934f2ebcb7eb57\MSVCR90.dll
ModLoad: 00000000`6d870000 00000000`6d8fe000   C:\Windows\WinSxS\x86_microsoft.vc90.crt_1fc8b3b9a1e18e3b_9.0.30729.6161_none_50934f2ebcb7eb57\MSVCP90.dll
ModLoad: 00000000`6c860000 00000000`6c88b000   C:\Windows\WinSxS\x86_microsoft.vc90.atl_1fc8b3b9a1e18e3b_9.0.30729.6161_none_51cd0a7abbe4e19b\ATL90.DLL
ModLoad: 00000000`66a00000 00000000`66e1a000   C:\PROGRA~2\COMMON~1\MICROS~1\OFFICE14\Cultures\office.odf
ModLoad: 00000000`65c80000 00000000`664e5000   C:\PROGRA~2\MICROS~4\Office14\1033\GrooveIntlResource.dll
ModLoad: 00000000`6bf70000 00000000`6bfe0000   ntshrui.dll
ModLoad: 00000000`6bf00000 00000000`6bf70000   C:\Windows\SysWOW64\ntshrui.dll
ModLoad: 00000000`71680000 00000000`7168b000   C:\Windows\SysWOW64\cscapi.dll
[JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "file:///C:/Software/Firefox%20Crashes/Firefox%20Crash.html" line: 0}]
(b84.1030): WOW64 breakpoint - code 4000001f (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for mozalloc.dll - 
mozalloc!mozalloc_abort+0x2b:
71521988 cc              int     3
0:000:x86> k
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for xul.dll - 
ChildEBP RetAddr  
WARNING: Stack unwind information not available. Following frames may be wrong.
003db584 64328ce1 mozalloc!mozalloc_abort+0x2b
003db5d4 642f57cb xul!imgLoader::SupportImageWithMimeType+0xe4cfd
003db644 642f3f57 xul!imgLoader::SupportImageWithMimeType+0xb17e7
003db688 642fc52f xul!imgLoader::SupportImageWithMimeType+0xaff73
003db77c 64155c90 xul!imgLoader::SupportImageWithMimeType+0xb854b
00000000 00000000 xul!CallWindowProcCrashProtected+0x58e2
0:000:x86> g
(b84.1030): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
mozalloc!mozalloc_abort+0x2e:
7152198b c705000000007b000000 mov dword ptr ds:[0],7Bh ds:002b:00000000=????????
0:000:x86> k
ChildEBP RetAddr  
WARNING: Stack unwind information not available. Following frames may be wrong.
003db584 64328ce1 mozalloc!mozalloc_abort+0x2e
003db5d4 642f57cb xul!imgLoader::SupportImageWithMimeType+0xe4cfd
003db644 642f3f57 xul!imgLoader::SupportImageWithMimeType+0xb17e7
003db688 642fc52f xul!imgLoader::SupportImageWithMimeType+0xaff73
003db77c 64155c90 xul!imgLoader::SupportImageWithMimeType+0xb854b
00000000 00000000 xul!CallWindowProcCrashProtected+0x58e2
0:000:x86> g
(b84.1030): Access violation - code c0000005 (!!! second chance !!!)
ntdll_774a0000!ZwRaiseException+0x12:
774c15de 83c404          add     esp,4
0:000:x86> k
ChildEBP RetAddr  
003db034 774b014d ntdll_774a0000!ZwRaiseException+0x12
003db140 6e9e3896 ntdll_774a0000!KiUserExceptionDispatcher+0x29
003db584 64328ce1 MSVCR100!fputs+0xfa [f:\dd\vctools\crt_bld\self_x86\crt\src\fputs.c @ 65]
WARNING: Stack unwind information not available. Following frames may be wrong.
003db5d4 642f57cb xul!imgLoader::SupportImageWithMimeType+0xe4cfd
003db644 642f3f57 xul!imgLoader::SupportImageWithMimeType+0xb17e7
003db688 642fc52f xul!imgLoader::SupportImageWithMimeType+0xaff73
003db77c 64155c90 xul!imgLoader::SupportImageWithMimeType+0xb854b
00000000 00000000 xul!CallWindowProcCrashProtected+0x58e2

Comment 3

[Loic]

Do you have crash reports in about:crashes?

Comment 4

[marekzmyslowski]

https://crash-stats.mozilla.com/report/index/bp-645daf49-fd4b-49c9-8c73-dfe312130819

Comment 5

[Paul Silaghi, QA [:pauly]]

Crashed FF 23.0.1 Win 7 x64 with the testcase: https://crash-stats.mozilla.com/report/index/21a24282-f7ba-4cb7-85be-d5dac2130826

Comment 6

[Boris Zbarsky [:bzbarsky]]

The crash in comment 5 and the info in comment 2 both point to an out-of-memory situation.  For the comment 5 crash, the attempted allocation was 598014368 bytes (so about 500MB).

The allocation seems to be this one:

  nsAutoArrayPtr<uint8_t> tmp(new uint8_t[aTarget->mImage->GetDataSize()]);

In SVGFEGaussianBlurElement::GaussianBlur.  This should be using fallible new, especially since it then null-checks the result!

Jonathan, you seem to have blame for this code... could you take a look, please?

Comment 7

[Robert Longson [:longsonr]]

Attachment: patch

Seems to me this is what we want.

Comment 8

[Jonathan Watt [:jwatt]]

Comment on attachment 795932 [details] [diff] [review]
patch

Thanks, Robert.

Comment 9

[Robert Longson [:longsonr]]

https://tbpl.mozilla.org/?tree=Try&rev=e14ab7fd1e52

Comment 10

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6537b89b9063

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6537b89b9063

Comment 12

[Paul Silaghi, QA [:pauly]]

No crashes after the fix, but Firefox freezes when loading the testcase. Is it ok?
26.0a1 (2013-09-09)

Comment 13

[Robert Longson [:longsonr]]

Yes, the page has a meta refresh to refresh every 2 seconds and it probably takes around 2 seconds to render because of the complicated filter chain.

Comment 14

[Paul Silaghi, QA [:pauly]]

Verified fixed based on comment 13.