rime / weasel

【小狼毫】Rime for Windows
https://rime.im
GNU General Public License v3.0
4.43k stars 542 forks source link

Crash caused by NULL pointer dereference, probably in `CInsertTextEditSession::DoEditSession()` #1100

Closed Smackd0wn closed 5 months ago

Smackd0wn commented 7 months ago

上报前请检查

操作系统信息

描述遇到的问题 My browser, Pale Moon, crashed on weasel+0x4b4f:

rcx=0000000000000000 (from crash dump)
00007ffb`06dd4b4f 488b01          mov     rax,qword ptr [rcx]

复现步骤 It crashed for a few times, but I cannot find a way to reliably reproduce it, sorry.

其他补充说明 I believe the root is in CInsertTextEditSession::DoEditSession(), where _pComposition is not checked against NULL:

https://github.com/rime/weasel/blob/399193512b2c30c985b85ef878d8f8e5c46af9c2/WeaselTSF/Composition.cpp#L332-L340

Whereas in CEndCompositionEditSession::DoEditSession(), there is a check, probably from the fix of issue #129 https://github.com/rime/weasel/blob/399193512b2c30c985b85ef878d8f8e5c46af9c2/WeaselTSF/Composition.cpp#L100-L110

I have no idea why _pComposition sometimes can be NULL, but it clearly needs to be checked before access, or make sure it cannot be NULL. Not sure if other DoEditSession() functions have similar issues.

P.S. the comment above ("Clear the dummy text") probably should be placed immediately above _ClearCompositionDisplayAttributes.

Techince commented 7 months ago

似乎只有Firefox系的浏览器才会触发此问题。 触发此问题的原因不得而知,但可以在执行WeaselTSF::_StartComposition里检测pContext->RequestEditSession返回值和输出值「hr」来预防此问题。 若系统是64位,可以测试一下此版本: 小狼毫64位

Smackd0wn commented 7 months ago

似乎只有Firefox系的浏览器才会触发此问题。 触发此问题的原因不得而知,但可以在执行WeaselTSF::_StartComposition里检测pContext->RequestEditSession返回值和输出值「hr」来预防此问题。

我只在 Pale Moon 里触发过此问题(在微信网页版微信中经常触发),我很少使用 Firefox 系的其他浏览器。我也不清楚触发问题的原因,但我想至少可以在 CInsertTextEditSession::DoEditSession() 的开头加上一行 if (_pComposition == nullptr) return E_FAIL; 来避免致使其他程序崩溃的问题。

我不了解 TSF,或许您提到的方法也可以保证 _pComposition 不是 NULL,但在 CInsertTextEditSession::DoEditSession() 里加上 NULL 指针的检查也没什么坏处,正如在 CEndCompositionEditSession::DoEditSession() 的开头就有类似的检查。

感谢提供您的 fork 版本,但我没有测试。您的源码与本仓库有数万行的区别,我也不确定这些变更是否会被 merge 到该官方版本里。

如果我上面提到的 workaround 可被接受,解决崩溃的问题只需增加一行代码。

P.S. 我不确定 CInlinePreeditEditSession::DoEditSession() 里是否也需要如此检查,但我即使开启了 inline_preedit,也会在 weasel+0x4b4f 处崩溃,即 CInsertTextEditSession::DoEditSession()

fxliang commented 5 months ago

0507bf7bb4dda862bacb615ca009636f8605a872 已添加nullptr检查 @Smackd0wn

Smackd0wn commented 5 months ago

感谢! @fxliang