Fixed a bug that causes the whole thunderbird in windows platform to crash

[huangcheng]

Hi, I found a null pointer dereference bug that causes thunderbird to crash, here is the code I digested from WinDbg

(37bc.15c4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
*** WARNING: Unable to verify checksum for ToastNotification.dll
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for ToastNotification.dll - 
eax=132587c8 ebx=00000009 ecx=00000000 edx=00000000 esi=132587c8 edi=00000002
eip=170d1110 esp=00a7bc78 ebp=00a7bc84 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00210202
ToastNotification+0x1110:
170d1110 668b02          mov     ax,word ptr [edx]        ds:002b:00000000=????

00 00a7bc84 170d1b65 00000000 0dc56660 0dc56680 ToastNotification+0x1110
01 00a7bca4 0465b85d 00000000 0dc56660 0dc56680 ToastNotification!DisplayToastNotification+0x75
02 00a7bcc4 0465b250 0465b2e0 00a7bcf4 00000002 xul!ffi_call_win32+0x35
03 00a7bd04 045071f2 0de05580 170d1af0 05b057b4 xul!ffi_call+0xf0
04 00a7be34 0448a764 05b4f280 00000006 00a7c0b0 xul!js::ctypes::FunctionType::Call+0x452 (FPO: [3,67,0]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\ctypes\ctypes.cpp @ 5934]
05 00a7c054 0448a60f 05b4f280 00a7c0c0 00000006 xul!js::Invoke+0x104 (FPO: [4,129,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 491]
06 00a7c0f0 043e401d 05b4f280 0d00a908 00a7c138 xul!js::Invoke+0x1df (FPO: [6,30,0]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 554]
07 00a7c13c 043e4253 05b4f280 00a7c188 00a7c178 xul!js::CrossCompartmentWrapper::call+0xdd (FPO: [3,9,4]) (CONV: thiscall) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\crosscompartmentwrapper.cpp @ 286]
08 00a7c164 043edd86 05b4f280 00a7c188 00a7c178 xul!js::Proxy::call+0xc3 (FPO: [3,3,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\proxy.cpp @ 382]
09 00a7c188 0448a764 05b4f280 00000006 0d00a900 xul!js::proxy_Call+0x46 (FPO: [3,5,0]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\proxy.cpp @ 693]
0a 00a7c3a8 044877e0 05b4f280 0d00a910 00000006 xul!js::Invoke+0x104 (FPO: [4,129,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 491]
0b 00a7cbd4 0448ecd1 05b4f280 00a7cc24 00000000 xul!Interpret+0x4d00 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 2602]
0c 00a7cbf8 0448a834 05b4f280 00a7cc24 0d00a890 xul!js::RunScript+0x161 (FPO: [2,3,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 448]
0d 00a7ce14 0448a60f 05b4f280 00a7ce80 00000006 xul!js::Invoke+0x1d4 (FPO: [4,129,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 520]
0e 00a7ceb0 043e401d 05b4f280 0d00a858 00a7cef8 xul!js::Invoke+0x1df (FPO: [6,30,0]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\vm\interpreter.cpp @ 554]
0f 00a7cefc 043e4253 05b4f280 00a7cf48 00a7cf38 xul!js::CrossCompartmentWrapper::call+0xdd (FPO: [3,9,4]) (CONV: thiscall) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\crosscompartmentwrapper.cpp @ 286]
10 00a7cf24 043edd86 05b4f280 00a7cf48 00a7cf38 xul!js::Proxy::call+0xc3 (FPO: [3,3,4]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\proxy.cpp @ 382]
11 00a7cf48 0448a764 05b4f280 00000006 0d00a850 xul!js::proxy_Call+0x46 (FPO: [3,5,0]) (CONV: cdecl) [c:\builds\moz2_slave\tb-rel-c-esr38-w32_bld-0000000\build\mozilla\js\src\proxy\proxy.cpp @ 693]

0:000> u ToastNotification+0x1100 L10
ToastNotification+0x1100:
170d1100 55              push    ebp
170d1101 8bec            mov     ebp,esp
170d1103 53              push    ebx
170d1104 56              push    esi
170d1105 8bf1            mov     esi,ecx
170d1107 8b4d08          mov     ecx,dword ptr [ebp+8]
170d110a 8bd1            mov     edx,ecx
170d110c 57              push    edi
170d110d 8d7a02          lea     edi,[edx+2]
170d1110 668b02          mov     ax,word ptr [edx]
170d1113 83c202          add     edx,2
170d1116 6685c0          test    ax,ax

According to ToastNotification+0x1100 L10, I finally targeted to the function

bool
ToastNotificationHandler::Init(const wchar_t* aImage, const wchar_t* aTitle, const wchar_t* aMessage, const wchar_t* aName);

wich called by

bool
DisplayToastNotification(const wchar_t* aImage, const wchar_t* aTitle, const wchar_t* aMessage, const wchar_t* aName, void* aCallbackActive, void* aCallbackDismiss)

Well, the original message is

0:000> du poi(@ebp + C)
0dc56660  "SOGO : Contacts"

0:000> du poi(@ebp + 10)
0dc56680  "No changes."

0:000> du poi(@ebp + 14)
0dc566a0  "Thunderbird"

0:000> dd @ebp + 8 L 1
00a7bc8c  00000000

As you can see, this message does not have an icon, which means the first parameter 'aImage' is set to null but the function does not check this, so the whole application crashes.

The fellowing is the capture of the message after I added some parameter checks.

[mkiol]

Thank you for the troubleshooting you've done! What should I do to reproduce the crash issue? I want to test it my self.

[huangcheng]

Well, I have many accounts added into my Thunderbird, and have GNotifier and Inverse SOGo Connector installed, when there are many notifications generated for example when the network is borken, Thunderbird would report can not connect to email servers, the bug will occur.

On 2016-04-27 14:10, Mkiol wrote:

Thank you for the troubleshooting you've done! What should I do to reproduce the crash issue? I want to test it my self.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/mkiol/GNotifier/pull/105#issuecomment-214980518

[mkiol]

What is the Gnotifier version that caused FF to crash?

I'm curious because in 43850c1bb3d3d57c52483dc9b636d97701b6bc3f I did a fix for SOGo Connector. There is additional check in main.js. If imageUrl is NULL, it will be replaced with the empty string.

if (imageUrl === null) {
  GNotifier_AlertsService_showAlertNotification_cb("");
  return;
}

[huangcheng]

This version of GNotifier is the one I'm in use and seems to be the newest in AMO.

[mkiol]

Thanks for the fix.