Bug 1264813 - ggz-base-libs: does not disable internal XML entity expansion
ggz-base-libs: does not disable internal XML entity expansion
Status: NEW
Product: Fedora
Classification: Fedora
Component: ggz-base-libs (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
: FutureFeature, Security
Depends On:
Blocks: 888729
  Show dependency treegraph
 
Reported: 2015-09-21 05:33 EDT by Florian Weimer
Modified: 2016-04-26 04:38 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: 889142
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Florian Weimer 2015-09-21 05:33:07 EDT
+++ This bug was initially created as a clone of Bug #889142 +++

ggz-base-libs 0.99.5 contains the following code in ggzcore/netxml.c:

        if (!(net->parser = XML_ParserCreate("UTF-8")))
                ggz_error_sys_exit
                    ("Couldn't allocate memory for XML parser");

        /* Setup handlers for tags */
        XML_SetElementHandler(net->parser, (XML_StartElementHandler)
                              _ggzcore_net_parse_start_tag,
                              (XML_EndElementHandler)
                              _ggzcore_net_parse_end_tag);
        XML_SetCharacterDataHandler(net->parser, _ggzcore_net_parse_text);
        XML_SetUserData(net->parser, net);

This does not disable expansion of XML entities in the internal DTD subset, making the code subject to denial-of-service attacks ("billion laughs").  It seems the data comes from the network, so a trust boundary is crossed, making this a (low impact) security issue.

Adding the following handler using

  XML_SetEntityDeclHandler(expat, EntityDeclHandler);

should be sufficient to address this issue.

// Stop the parser when an entity declaration is encountered.
static void
EntityDeclHandler(void *userData,
		  const XML_Char *entityName, int is_parameter_entity,
		  const XML_Char *value, int value_length,
		  const XML_Char *base, const XML_Char *systemId,
		  const XML_Char *publicId, const XML_Char *notationName)
{
  XML_StopParser((XML_Parser)userData, XML_FALSE);
}
Comment 1 Jan Kurik 2016-02-24 08:46:49 EST
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase
Comment 2 Fedora Admin XMLRPC Client 2016-04-21 15:17:10 EDT
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.
Comment 3 Fedora Admin XMLRPC Client 2016-04-26 03:38:41 EDT
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.
Comment 4 Hans de Goede 2016-04-26 03:42:36 EDT
ggzcore's xml parsing is actually intended to parse xml coming from the network, so I'm afraid that disallowing network based dtd-s may break things ...
Comment 5 Florian Weimer 2016-04-26 04:13:21 EDT
(In reply to Hans de Goede from comment #4)
> ggzcore's xml parsing is actually intended to parse xml coming from the
> network, so I'm afraid that disallowing network based dtd-s may break things
> ...

Do you expect these XML documents contain an internal DTD subset?
Comment 6 Hans de Goede 2016-04-26 04:38:51 EDT
(In reply to Florian Weimer from comment #5)
> (In reply to Hans de Goede from comment #4)
> > ggzcore's xml parsing is actually intended to parse xml coming from the
> > network, so I'm afraid that disallowing network based dtd-s may break things
> > ...
> 
> Do you expect these XML documents contain an internal DTD subset?

XML is not really my forte so I do not know, I just picked up ggz because it is a dep of a couple of games I [co-]maintain. Looking at the code / protocol it does not seem to use DTDs at all, the code in question is for parsing a streaming XML protocol with messages like this:

http://www.ggzgamingzone.org/docs/spec/html/req_login.html

So I think that the fix you suggested is safe and should not lead to regressions, if you think the same I'll go ahead and add your fix.

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