On Wed, Jun 10, 2009 at 7:22 PM, <bo...@apache.org> wrote: > Author: bojan > Date: Thu Jun 11 00:22:09 2009 > New Revision: 783589 > > URL: http://svn.apache.org/viewvc?rev=783589&view=rev > Log: > Backport r781403 from the trunk. > Prevent "billion laughs" attack against expat: > > * xml/apr_xml.c (entity_declaration, default_handler): Add new handlers > for expat 2.x and 1.x respectively. > (apr_xml_parser_create): Install handler to prevent expansion of > internal entities with expat 1.x, and to fail on an entity > declaration with expat 2.x. > > * test/testxml.c (create_dummy_file, dump_xml): Test that predefined > entities are expanded. > (test_billion_laughs): New test case. > > Added: > apr/apr-util/branches/1.4.x/test/data/billion-laughs.xml > - copied unchanged from r781403, > apr/apr/trunk/test/data/billion-laughs.xml > Modified: > apr/apr-util/branches/1.4.x/ (props changed) > apr/apr-util/branches/1.4.x/CHANGES > apr/apr-util/branches/1.4.x/buckets/apr_brigade.c (props changed) > apr/apr-util/branches/1.4.x/test/testxml.c > apr/apr-util/branches/1.4.x/xml/apr_xml.c
> Modified: apr/apr-util/branches/1.4.x/test/testxml.c > URL: > http://svn.apache.org/viewvc/apr/apr-util/branches/1.4.x/test/testxml.c?rev=783589&r1=783588&r2=783589&view=diff > ============================================================================== > --- apr/apr-util/branches/1.4.x/test/testxml.c (original) > +++ apr/apr-util/branches/1.4.x/test/testxml.c Thu Jun 11 00:22:09 2009 ... > @@ -149,11 +148,29 @@ > ABTS_TRUE(tc, rv != APR_SUCCESS); > } > > +static void test_billion_laughs(abts_case *tc, void *data) > +{ > + apr_file_t *fd; > + apr_xml_parser *parser; > + apr_xml_doc *doc; > + apr_status_t rv; > + > + rv = apr_file_open(&fd, "billion-laughs.xml", > + APR_FOPEN_READ, 0, p); > + apr_assert_success(tc, "open billion-laughs.xml", rv); > + > + rv = apr_xml_parse_file(p, &parser, &doc, fd, 2000); > + ABTS_TRUE(tc, rv != APR_SUCCESS); if I understand correctly: with expat >= 2.x, we are able to force a parse failure, so rv shouldn't be APR_SUCCESS with expat 1.x, we aren't able to force a parse failure but we prevent expansion, and rv will be APR_SUCCESS apr-util 1.4.x bundles expat 1.x (which perhaps few people use?), and running this test fails since rv is APR_SUCCESS Is it reasonable to leave the apr-util 1.4.x test like 1.3's and ignore the return code? /* Don't test for return value; if it returns, chances are the bug * is fixed or the machine has insane amounts of RAM. */ apr_xml_parse_file(p, &parser, &doc, fd, 2000);
