On Mon, Nov 16, 2009 at 3:43 PM, Jeff Trawick <[email protected]> wrote: > On Wed, Jun 10, 2009 at 7:22 PM, <[email protected]> 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); >
I'm changing the test in 1.4.x's testxml.c to match 1.3.x's unless someone objects Real Soon Now.
