Page MenuHomeWildfire Games

Update OSX libxml2 developmental snapshot to prevent potential exploits
AbandonedPublic

Authored by elexis on Jul 1 2017, 2:40 AM.

Details

Reviewers
None
Trac Tickets
#4362
Summary

As mentioned in https://code.wildfiregames.com/D679?id=2703#inline-12860 there are four exploitable vulnerabilities in 2.9.4 that most likely are not applicable to 0AD,
but could be fixed preemptively by using a developmental snapshot instead of the latest release.

Afaics, these two commits fix those 4 CVEs CVE-2017-9050 CVE-2017-9049 CVE-2017-9048 CVE-2017-9047
https://git.gnome.org/browse/libxml2/commit/?id=e26630548e7d138d2c560844c43820b6767251e3
https://git.gnome.org/browse/libxml2/commit/?id=932cc9896ab41475d4aa429c27d9afd175959d74

Since 932cc9896ab41475d4aa429c27d9afd175959d74 is the more recent commit, so that snapshot contains both fixes. Confirm by checking for the presence of a new file added by both commits.

Test Plan

Test compilation. Open the structure tree to check whether it falls apart for any reason.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2483
Build 4184: Vulcan LintJenkins
Build 4183: Vulcan BuildJenkins
Build 4182: arc lint + arc unit

Event Timeline

elexis created this revision.Jul 1 2017, 2:40 AM
elexis edited the summary of this revision. (Show Details)Jul 1 2017, 2:43 AM

Judging from https://git.gnome.org/browse/libxml2/log/ a slightly more recent version might prevent a few memory leaks, which most likely also don't directly affect us (but I'm not exactly sure what the few libxml2 functions we use call internally). So it might be nice to also include those when we are going for actually fixing those issues.

(Unrelated, but we might want to also update libraries for Windows.)

libraries/osx/build-osx-libs.sh
26

Might be nice to mention that this is a snapshot some time after 2.9.4 but before whatever is the next release (probably 2.9.5).

Vulcan added a subscriber: Vulcan.Jul 1 2017, 3:44 AM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/274/ for more details.

Vulcan added a comment.Jul 1 2017, 5:33 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1675/ for more details.

In D699#27874, @leper wrote:

we might want to also update libraries for Windows

Someone might want to propose that in the staff meeting tomorrow whose goal is to determine CF.

elexis added a comment.Jul 4 2017, 1:32 AM
In D699#27874, @leper wrote:

Judging from https://git.gnome.org/browse/libxml2/log/ a slightly more recent version might prevent a few memory leaks, which most likely also don't directly affect us (but I'm not exactly sure what the few libxml2 functions we use call internally). So it might be nice to also include those when we are going for actually fixing those issues.

How do we know though that there aren't random bugs in that by definition not stable snapshot or WIP grade commits?

(Unrelated, but we might want to also update libraries for Windows.)

Someone might want to propose that in the staff meeting tomorrow whose goal is to determine CF.

Transported the message.
Brief analysis of the current state of windows libraries at https://trac.wildfiregames.com/ticket/3004#comment:27
Not convinced that those exploits nor the one in this patch should be release blockers at this stage of the release. Seems unlikely that someone can do something with these vulns.
Do agree about the need to nurse the dlls at a time before feature freeze.

If the snapshot version in the patch should be updated to a more recent one due to a memory leak, then we should just go all the way to the most recent development snapshot it sounds:

In https://git.gnome.org/browse/libxml2/log/

17 hours	Heap-buffer-overflow read of size 1 in xmlFAParsePosCharGroupHEADmaster	David Kilzer	1	-1/+1
17 hours	Fix NULL pointer deref in xmlFAParseCharClassEsc	Nick Wellnhofer	1	-1/+2
17 hours	Fix infinite loops with push parser in recovery mode

Didn't really make me more confident that this is guaranteed to be a better option than using the most recent stable.

elexis abandoned this revision.Jul 6 2017, 8:05 PM

Committing the patch was discussed with Itms, but we are not convinced of doing so for several less significant reasons:

  • Noone to test the patch properly and the library is less tested than the most recent stable release
  • The windows release won't be fixed either and that has a higher attack surface.
  • Someone would have to craft a malicious map file that would have to download malicious code and talk people into downloading and installing it. Since maps can also contain code too, the attacker doesn't need the exploit to cause harm.
In D699#28346, @elexis wrote:

Committing the patch was discussed with Itms, but we are not convinced of doing so for several less significant reasons:

  • Noone to test the patch properly and the library is less tested than the most recent stable release

Ah, the joys of supporting OS X. I'll take obscure *nix over that most of the time.

  • The windows release won't be fixed either and that has a higher attack surface.

(Not that I care about any of those two, but [see below])

  • Someone would have to craft a malicious map file that would have to download malicious code and talk people into downloading and installing it. Since maps can also contain code too, the attacker doesn't need the exploit to cause harm.

Probably, but there are ideas about having sort of automated map/mod sharing and unless we start caring about things like this I'll try to shoot those plans down before anyone gets a chance to defend those plans. Not that I care a lot, but at the point we start doing this I will have to show that I have got nothing to do with those releases.

But to be a little less alarmed, I did warn you of possible issues.

elexis added a comment.Jul 8 2017, 1:26 AM

WFG should review and whitelist mod versions. Other mods should at most be accessible with a red warning message box with a red stop sign.

It was discussed on http://irclogs.wildfiregames.com/2017-07-05-QuakeNet-%230ad-dev.log

21:53 < echotangoecho> we can exploit arbitrary code execution vulnerabilities by specially crafted js
21:54 < echotangoecho> search for code execution vulnerabilities back to spidermonkey 38 -- there are lots
21:55 < echotangoecho> (ok, they are hard to find as most are listed under mozilla ff)

22:42 < Philip`> I think you need to consider what sources of data should be trusted (i.e. you assume they are non-malicious) and what are untrusted, and which libraries deal with each type of data
22:43 < Philip`> e.g. I think mods pretty much have to be trusted - it's futile to try to make the engine handle malicious mods safely, because the attack surface is massive, so you shouldn't even try
22:44 < Philip`> whereas data received from a game server over the network should be untrusted and should never be able to compromise the user's computer (which is still difficult to guarantee but kind of feasible)
22:45 < Philip`> (Mods can e.g. trivially crash the game by calling Engine.Crash() from a GUI script; it's totally not designed to run scripts securely)
22:45 < Philip`> (and so it's no worse if a malicious Collada file can crash the game)

and with Itms starting 17:40 in http://irclogs.wildfiregames.com/2017-07-06-QuakeNet-%230ad-dev.log