Bug 94275 - Unguarded strlen causes core dump when XKeysymToString returns NULL
Summary: Unguarded strlen causes core dump when XKeysymToString returns NULL
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.0.1.1 rc
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:5.1.0 target:5.0.3
Keywords: haveBacktrace
Depends on:
Blocks:
 
Reported: 2015-09-16 12:42 UTC by Alex Bennée
Modified: 2016-10-25 19:20 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
ODT export of ORG export which causes crash (2.41 MB, application/vnd.oasis.opendocument.text)
2015-09-16 12:42 UTC, Alex Bennée
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Bennée 2015-09-16 12:42:12 UTC
Created attachment 118768 [details]
ODT export of ORG export which causes crash

I thought I'd test importing an ODT document generated from Emacs' org-mode export. Unfortunately it causes a core dump when an unguarded strlen in SalDisplay::GetKeyNameFromKeySym attempts to operate on a NULL return.

Backtrace:

Program received signal SIGSEGV, Segmentation fault.
strlen () at ../sysdeps/x86_64/strlen.S:106
106     ../sysdeps/x86_64/strlen.S: No such file or directory.
(gdb) bt
#0  strlen () at ../sysdeps/x86_64/strlen.S:106
#1  0x00007fffdfd69083 in SalDisplay::GetKeyNameFromKeySym (this=this@entry=0x1121b30, nKeySym=<optimised out>)
    at /build/libreoffice-Ke3JzN/libreoffice-5.0.1~rc2/vcl/unx/generic/app/saldisp.cxx:744
#2  0x00007fffdfd696f5 in SalDisplay::GetKeyName (this=0x1121b30, nKeyCode=nKeyCode@entry=9476) at /build/libreoffice-Ke3JzN/libreoffice-5.0.1~rc2/vcl/unx/generic/app/saldisp.cxx:798
#3  0x00007fffe17892a2 in GtkSalFrame::GetKeyName (this=<optimised out>, nKeyCode=<optimised out>) at /build/libreoffice-Ke3JzN/libreoffice-5.0.1~rc2/vcl/unx/gtk/window/gtksalframe.cxx:3005
#4  0x00007ffff6125bbc in vcl::KeyCode::GetName (this=this@entry=0x7fffffffbfb0, pWindow=<optimised out>, pWindow@entry=0x0)
    at /build/libreoffice-Ke3JzN/libreoffice-5.0.1~rc2/vcl/source/window/keycod.cxx:108
...


(gdb) directory ~/disk/packages/libreoffice-5.0.1~rc2/vcl/unx/generic/app/
Source directories searched: /home/alex/disk/packages/libreoffice-5.0.1~rc2/vcl/unx/generic/app:/home/alex/disk/packages/libreoffice-5.0.1~rc2:$cdir:$cwd
(gdb) l
739             {
740                 aRet = ::vcl_sal::getKeysymReplacementName( aLang, nKeySym );
741                 if( aRet.isEmpty() )
742                 {
743                     const char *pString = XKeysymToString( nKeySym );
744                     int n = strlen( pString );
745                     if( n > 2 && pString[n-2] == '_' )
746                         aRet = OUString( pString, n-2, RTL_TEXTENCODING_ISO_8859_1 );
747                     else
748                         aRet = OUString( pString, n, RTL_TEXTENCODING_ISO_8859_1 );
(gdb) info locals
pString = 0x0
n = <optimised out>
aLang = "en"
aRet = ""
aKeyCode = <optimised out>

I'm not sure which element of the document it was failing on as optimization has hidden the useful variables.
Comment 1 Alex Bennée 2015-09-16 12:43:48 UTC
I was running the Ubuntu LibreOffice PPA but I can see the problem is still there in the latest GIT:

  https://github.com/LibreOffice/core/blob/master/vcl/unx/generic/app/saldisp.cxx#L743
Comment 2 Julien Nabet 2015-09-16 19:40:49 UTC
On pc Debian x86-64 with LO Debian package 5.0.1.2, the doc doesn't crash when I open it and scroll it to the end.
Idem with master sources updated today.

Did I miss something?

BTW, official git repos are:
git://anongit.freedesktop.org/libreoffice/core (for just reading)
git://gerrit.libreoffice.org/core (for those who have access in read/write)
Comment 3 Alex Bennée 2015-09-18 10:47:41 UTC
I suspect it will depend on system setup. The key thing is the call to  XKeysymToString can return NULL and strlen isn't safe to call on null:

    http://www.x.org/archive/current/doc/man/man3/XStringToKeysym.3.xhtml
Comment 4 Julien Nabet 2015-09-18 20:53:20 UTC
Re reading the source, you're indeed right.
I submitted a patch to gerrit review here (master sources only for the moment)
See https://gerrit.libreoffice.org/18708
Comment 5 Commit Notification 2015-09-21 12:02:13 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e7ff9ade29aff212b2e2728a15bbf5a11f5b8717

tdf#94275: core dump when XKeysymToString returns NULL

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2015-09-21 13:09:36 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cc7edea1be64c2050664b9a4b40192baa3d067ea&h=libreoffice-5-0

tdf#94275: core dump when XKeysymToString returns NULL

It will be available in 5.0.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.