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: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Libreoffice (show other bugs)
Version: 4.4.0.0.alpha0+ Master
Hardware: Other All
: high major
Assignee: Matthew Francis
QA Contact:
URL:
Whiteboard: target:4.4.0
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-19 08:49 UTC by Matthew Francis
Modified: 2014-09-22 05:17 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


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

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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.