Bug 459830 - latest libxml2 advisory breaks gnome initialization (gnome-panel?, nautilus?, gconf?)
latest libxml2 advisory breaks gnome initialization (gnome-panel?, nautilus?,...
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: libxml2 (Show other bugs)
5.2
All Linux
medium Severity medium
: rc
: ---
Assigned To: Daniel Veillard
: Regression
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-22 14:56 EDT by Ben Levenson
Modified: 2009-08-14 04:03 EDT (History)
7 users (show)

See Also:
Fixed In Version: 2.6.26-2.1.2.4
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-29 07:23:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Example of patch for upstream (6.98 KB, patch)
2008-08-22 19:37 EDT, Daniel Veillard
no flags Details | Diff
another patch related to upstream but using the owner field of the entity structure (8.09 KB, patch)
2008-08-23 05:07 EDT, Daniel Veillard
no flags Details | Diff
New patch for RHEL-5 reusing the owner field (8.05 KB, patch)
2008-08-23 06:05 EDT, Daniel Veillard
no flags Details | Diff
New patch for RHEL-4 reusing the owner field (8.05 KB, patch)
2008-08-23 06:23 EDT, Daniel Veillard
no flags Details | Diff
New patch for RHEL-3 reusing the owner field (8.04 KB, patch)
2008-08-25 04:06 EDT, Daniel Veillard
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
GNOME Desktop 549087 None None None Never

  None (edit)
Description Ben Levenson 2008-08-22 14:56:23 EDT
Description of problem:
latest libxml2 advisory breaks gnome initialization (gnome-panel?, nautilus?, gconf?)

I'm still in the early stages of debugging this, but I can't get a fully operational gnome-session up and running after upgrading to libxml2-2.6.26-2.1.2.3

Everything works as expected if I roll back to libxml2-2.6.26-2.1.2.1.
Comment 1 Ray Strode [halfline] 2008-08-22 16:32:51 EDT
So the patch has this bit:

--- include/libxml/parser.h     (revision 3771)
+++ include/libxml/parser.h     (working copy)
@@ -297,6 +297,7 @@ struct _xmlParserCtxt {
      */
     xmlError          lastError;
     xmlParserMode     parseMode;    /* the parser mode */
+    unsigned long    nbentities;    /* number of entities references */
 };
 
 /**
--- include/libxml/entities.h.orig      2005-01-04 15:49:49.000000000 +0100
+++ include/libxml/entities.h   2008-08-11 17:56:53.000000000 +0200
@@ -56,6 +56,7 @@ struct _xmlEntity {
     struct _xmlEntity     *nexte;      /* unused */
     const xmlChar           *URI;      /* the full URI as computed */
     int                    owner;      /* does the entity own the childrens */
+    unsigned long     nbentities;      /* the number of entities references */
 };
 
Which is growing a structure that's in a public header file.

librsvg is doing some iffy code:

       entity = xmlMalloc (sizeof(xmlEntity));
       memset(entity, 0, sizeof(xmlEntity));
       entity->type = XML_ENTITY_DECL;
       dupname = (xmlChar *) xmlMemStrdup ((const char *)name);
       entity->name = dupname; 
       
       entity->etype = type;

in a function called rsvg_entity_decl

It should probably be using some constructor api instead (xmlAddEntity?), but we can't break ABI in RHEL. It might break existing customers systems.

I'd recommend this errata getting fixed a different way that doesn't change abi.
Comment 3 Daniel Veillard 2008-08-22 18:42:22 EDT
Oh crap !!!
there are 
  xmlAddDocEntity() and xmlAddDtdEntity() which do this kind of things
in the API.
Creation of entities really should be done only by an XML parser in 
the DTD parsing stage, no application code should ever create some
unless they buid a document with a DTD, and those entry points are for this.

Daniel
Comment 4 Daniel Veillard 2008-08-22 18:53:50 EDT
Really this is a code abuse, there are constructor for those,
and growing a structure is usually safe, the patch also grows the
parser structure which is also public. And there was no way to make
a clean handling of that DoS without growing the structures to add
additional informations.
I first tried a simpler, less intrusive approach, but this still would
not catch some of the entities recursions exposed.
Absolute ABI maintainance for libxml2 is near impossible for the simple
reason that (nearly) everything is exposed.
In a nutshell I don't have a solution which will work reliably without
extending those 2 structures. I have tried, really,

Daniel
Comment 5 Ben Levenson 2008-08-22 19:33:14 EDT
Looks like this is restricted to the following icon set:
/usr/share/icons/HighContrast-SVG


backtrace from gdb 'eog HighContrast.svg' (Thanks dmalcolm!):
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1114208576 (LWP 5615)]
0x00000035c4871033 in _int_free () from /lib64/libc.so.6
(gdb) bt
#0  0x00000035c4871033 in _int_free () from /lib64/libc.so.6
#1  0x00000035c4874c5c in free () from /lib64/libc.so.6
#2  0x00002ab9acd4da4c in xmlParseEntityDecl () from /usr/lib64/libxml2.so.2
#3  0x00002ab9acd4e973 in xmlParseMarkupDecl () from /usr/lib64/libxml2.so.2
#4  0x00002ab9acd4ea0e in xmlParseMarkupDecl () from /usr/lib64/libxml2.so.2
#5  0x00002ab9acd53118 in xmlParseChunk () from /usr/lib64/libxml2.so.2
#6  0x00002aaaaacfe8e0 in rsvg_error_quark () from /usr/lib64/librsvg-2.so.2
#7  0x00002aaaaaab8d3c in fill_info () from
/usr/lib64/gtk-2.0/2.10.0/loaders/svg_loader.so
#8  0x00000035c94098d4 in gdk_pixbuf_loader_write () from
/usr/lib64/libgdk_pixbuf-2.0.so.0
#9  0x000000000041ef1b in g_cclosure_marshal_VOID__VOID ()
#10 0x000000000041a156 in g_cclosure_marshal_VOID__VOID ()
#11 0x00000000004238f2 in g_cclosure_marshal_VOID__VOID ()
#12 0x000000000042345a in g_cclosure_marshal_VOID__VOID ()
#13 0x00000035c6048f24 in g_thread_create_full () from /lib64/libglib-2.0.so.0
#14 0x00000035c54062f7 in start_thread () from /lib64/libpthread.so.0
#15 0x00000035c48d1b6d in clone () from /lib64/libc.so.6
Comment 6 Daniel Veillard 2008-08-22 19:35:46 EDT
Okay I have tried to make a patch where the struct _xmlEntity
is not changed, the nbentities is piggy backed on the checked int
in the same structure which is currently used only as a boolean.
It still grows the parser context structure, but really I don't see how
user code could hand duplicate it or try to embed it in a structure !
patch against upstream available as a proof of concept. But this would
need to be backported to RHEL 5 (and possibly 4).
I will look at this again tomorrow.

Daniel
Comment 7 Ben Levenson 2008-08-22 19:36:46 EDT
> backtrace from gdb 'eog HighContrast.svg'
what I meant to say was "eog $(any icon from the HighContrast-SVG set)"
Comment 8 Daniel Veillard 2008-08-22 19:37:23 EDT
Created attachment 314849 [details]
Example of patch for upstream
Comment 9 Daniel Veillard 2008-08-22 19:40:05 EDT
w.r.t. comment #7, I'm afraid any icon containing an entity declaration
in their XML SVG will hit this. No idea what the rscvg had in mind, probably
they though that SAX parsing and doing entities their own way was a good idea.
Ben could you check if RHEL4 is affected too ?

  thanks,

Daniel
Comment 10 Daniel Veillard 2008-08-22 20:00:30 EDT
Damn the patch won't work for RHEL5, the struct  _xmlEntity don't
have a 'checked' field either. I don't see any way to piggy back the
information on another field, unless trying to reuse 'owner' which is
also used as a boolean, but its use is very distinct, and that could
really break programs which actually used the API correctly.

Daniel
Comment 11 Daniel Veillard 2008-08-23 05:07:12 EDT
Created attachment 314860 [details]
another patch related to upstream but using the owner field of the entity structure
Comment 12 Daniel Veillard 2008-08-23 05:14:23 EDT
that new patch reuses the owner field of the entity structure to count
the entities replacements needed to expand the current entity. The 
owner field was previously used only as 0 or 1 to indicate if the entity
subtree was owned by that entity node or a copy from another reference.
This makes that code even harder to follow but I think I checked all the 
cases possible, ran upstream regression tests and checked against memory 
leaks and it looks okay.
The patch still need to add a field to the parser context, but I think 
that can't be avoided, and this should be safe.
I note that the entity structures and the parser context have been grown
previously in the libxml2-2.6.x releases and that librsvg problem didn't
surfaced, I wonder if their upstream version has cleaned up that code.

I will now backport that new patch to RHEL-5 and RHEL-4 that should be 
relatively easy.

Daniel
Comment 13 Daniel Veillard 2008-08-23 06:05:07 EDT
Created attachment 314862 [details]
New patch for RHEL-5 reusing the owner field
Comment 14 Daniel Veillard 2008-08-23 06:23:32 EDT
Created attachment 314863 [details]
New patch for RHEL-4 reusing the owner field
Comment 16 Ben Levenson 2008-08-23 13:29:04 EDT
Severity/Priority = medium/medium

GNOME:
This will crash all Gnome apps that attempt to render an SVG with an entity declaration.  For example gnome-panel will crash if the default icon set is "HighContrast-SVG"; Nautilus will crash if thumbnails are enabled and a customer navigates to a directory with an SVG containing an entity declaration.

Customers experiencing problems with the latest libmxl2 can unset the HighContrast-SVG icon set by running the following command from a console:

gconftool-2 --unset /desktop/gnome/interface/icon_theme

If the customer's custom application launchers are impacted, they can work around this bug by specifying a new icon in the relevant .desktop files found here:

~/.gnome2/panel2.d/default/launchers/

Additionally, thumbnails can be disabled in Nautilus with the following command:

gconftool-2 --type string -s /apps/nautilus/preferences/show_image_thumbnails "never"

or by setting "Show Thumbnails" to "Never" in the following control panel:
System -> Preferences -> File Management -> Preview

KDE:
Unable to reproduce this bug in KDE.
Comment 17 Ben Levenson 2008-08-23 14:49:12 EDT
(In reply to comment #9)
> Ben could you check if RHEL4 is affected too ?

I can't reproduce this on RHEL-4 (verified on i386 and x86_64)
Comment 19 Daniel Veillard 2008-08-25 04:06:01 EDT
Created attachment 314908 [details]
New patch for RHEL-3 reusing the owner field
Comment 21 Dave Malcolm 2008-08-25 13:19:20 EDT
Runnning:
find /usr/share/icons -name \*.svg | xargs grep ENTITY | wc -l
on my RHEL 5 workstation shows 751 references to XML entities within svg files; all of these are within /usr/share/icons/HighContrast-SVG

Not all svg files within /usr/share/icons/HighContrast-SVG contain XML entities.

This is with gnome-themes-2.16.0-1.fc6
Comment 23 Pınar Yanardağ 2008-08-27 04:27:31 EDT
(In reply to comment #16)
> (...)
> KDE:
> Unable to reproduce this bug in KDE.

It seems Dolphin (in KDE 4.1) crashes, too (while navigating a directory icon.) The new patch solves this issue, though.
Comment 24 Tomas Hoger 2008-08-27 04:52:53 EDT
Correct, Dolphin / Strigi was mentioned in the Debian bug report:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=496125#99

Note You need to log in before you can comment on or make changes to this bug.