Bug Hunting Session
Bug 84086 - Find and fix anti-patterns that result in use-after-free of strings
Summary: Find and fix anti-patterns that result in use-after-free of strings
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other All
: high major
Assignee: Matthew Francis
URL:
Whiteboard: target:4.4.0
Keywords:
Depends on:
Blocks: Find-Search
  Show dependency treegraph
 
Reported: 2014-09-19 08:49 UTC by Matthew Francis
Modified: 2016-10-01 09:54 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Apparent bugs (2.17 KB, text/plain)
2014-09-19 14:41 UTC, Matthew Francis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Francis 2014-09-19 08:49:56 UTC
In the codebase there are currently some examples of code like this:

    gchar* aItemCommandStr = (gchar*) OUStringToOString( aItemCommand, RTL_TEXTENCODING_UTF8 ).getStr();

This fails as a pattern, because the destructor of the anonymous temporary OString is called at the end of this expression, before the gchar* that is returned can be used.

(the destructor is only called at the very end of the expression, so in this case it would suffice to wrap with a g_strdup() on the same line, or alternatively to split the expression into two with a named OString)

See bug 69090 for one example of this that resulted in a visible bug.


There may be other related issues of a similar nature. A clang plugin would potentially be a good way to guard against these.
Comment 1 Noel Grandin 2014-09-19 08:53:41 UTC
A git grep to find instances to verify looks like:

  git grep -P "\(\w+\s*\*\)\s*\(\w+\s*\*\)"

And there are approx. 101 places to check
Comment 2 Noel Grandin 2014-09-19 09:09:35 UTC
Oops, copied wrong thing - this is the correct search pattern

      git grep -iP "OUStringToOString.*getStr\(" 

and there are 952 locations to check.

But most of them can be eliminated with a second grep pass because they are calling the logging methods.
Comment 3 Matthew Francis 2014-09-19 14:41:12 UTC
Created attachment 106553 [details]
Apparent bugs

Although not a substitute for a clang plugin, a visual grep of the likely candidates revealed the attached

- including one comment on the basis that it's not nice to leave unexploded ordnance hanging around
- including several defines where a further search showed probable unsafe uses thereof (not listed separately)
Comment 4 Commit Notification 2014-09-22 05:17:01 UTC
Matthew J. Francis committed a patch related to this issue.
It has been pushed to "master":

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

fdo#84086 Fix assorted use-after-free bugs



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 5 QA Administrators 2015-10-14 19:56:46 UTC Comment hidden (obsolete)
Comment 6 Xisco Faulí 2016-09-15 22:17:17 UTC
Hello,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?