apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r783589 - in /apr/apr-util/branches/1.4.x: ./ CHANGES buckets/apr_brigade.c test/data/billion-laughs.xml test/testxml.c xml/apr_xml.c
Date Fri, 11 Dec 2009 15:45:49 GMT
On Mon, Nov 16, 2009 at 3:43 PM, Jeff Trawick <trawick@gmail.com> wrote:
> On Wed, Jun 10, 2009 at 7:22 PM,  <bojan@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);
>

I'm changing the test in 1.4.x's testxml.c to match 1.3.x's unless
someone objects Real Soon Now.

Mime
View raw message