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 Mon, 16 Nov 2009 20:43:36 GMT
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);

Mime
View raw message