Unrestricted XML entity expansion in mod_dontdothat and Subversion clients using http(s):// Summary: ======== Subversion's mod_dontdothat module and clients using http(s):// are vulnerable to a denial-of-service attack caused by exponential XML entity expansion. The attack, otherwise known as the "billion laughs attack", targets XML parsers and can cause the targeted process to consume an excessive amount of CPU resources or memory. There are no known instances of this problem being exploited in the wild. The details for this vulnerability have been disclosed on the Subversion development mailing list. Known vulnerable: ================= mod_dontdothat 1.4.0 through 1.8.16 (inclusive) mod_dontdothat 1.9.0 through 1.9.4 (inclusive) Subversion clients 1.4.0 through 1.8.16 (inclusive) Subversion clients 1.9.0 through 1.9.4 (inclusive) Note: Subversion clients 1.4.0 through 1.7.22 can use either Serf or Neon as HTTP library. Among these versions, only clients using Serf are vulnerable. Known fixed: ============ Subversion 1.8.17 Subversion 1.9.5 Subversion clients not using http(s):// are not vulnerable Details: ======== The attack takes advantage of three properties of XML (substitution entities, nested entities, and inline DTDs) that allow preparing an XML bomb -- a small block of XML that can require a significant amount of CPU resources or memory to process. An authenticated remote attacker can cause denial-of-service conditions on the server using mod_dontdothat by sending a specially crafted REPORT request. The attack does not require access to a particular repository. If an attacker has control over HTTP responses sent to a Subversion client, he can cause denial-of-service conditions on the client by injecting the XML bomb into the response. Severity: ========= CVSSv2 Base Score: 3.5 CVSSv2 Base Vector: AV:N/AC:M/Au:S/C:N/I:N/A:P We consider this to be a medium risk vulnerability. While mod_dontdothat is not typically installed, server installations using it are vulnerable to authenticated attackers. The attack does not require read access to a particular repository. Servers which allow for anonymous reads will be vulnerable without authentication. The client side of this vulnerability might be exploited as well, but requires an attacker to have control over HTTP responses delivered to the client. Recommendations: ================ We recommend all users to upgrade to Subversion 1.9.5. Users of Subversion 1.8.x and 1.9.x who are unable to upgrade may apply the included patch. New Subversion packages can be found at: http://subversion.apache.org/packages.html No workaround is available. References: =========== CVE-2016-8734 (Subversion) Reported by: ============ Florian Weimer, Red Hat, Inc. Patches: ======== Patch for Subversion 1.9.4: [[[ Index: subversion/libsvn_ra_serf/xml.c =================================================================== --- subversion/libsvn_ra_serf/xml.c (revision 1768981) +++ subversion/libsvn_ra_serf/xml.c (working copy) @@ -988,7 +988,31 @@ #endif } +#if XML_VERSION_AT_LEAST(1, 95, 8) +static void +expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + struct expat_ctx_t *ectx = userData; + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(ectx->parser, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void +expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif + /* Implements svn_ra_serf__response_handler_t */ static svn_error_t * expat_response_handler(serf_request_t *request, @@ -1042,6 +1066,12 @@ XML_SetUserData(ectx->parser, ectx); XML_SetElementHandler(ectx->parser, expat_start, expat_end); XML_SetCharacterDataHandler(ectx->parser, expat_cdata); + +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(ectx->parser, expat_entity_declaration); +#else + XML_SetDefaultHandler(ectx->parser, expat_default_handler); +#endif } while (1) Index: subversion/libsvn_subr/xml.c =================================================================== --- subversion/libsvn_subr/xml.c (revision 1768981) +++ subversion/libsvn_subr/xml.c (working copy) @@ -46,6 +46,14 @@ #error Expat is unusable -- it has been compiled for wide characters #endif +#ifndef XML_VERSION_AT_LEAST +#define XML_VERSION_AT_LEAST(major,minor,patch) \ +(((major) < XML_MAJOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) < XML_MINOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) == XML_MINOR_VERSION && \ + (patch) <= XML_MICRO_VERSION)) +#endif /* XML_VERSION_AT_LEAST */ + const char * svn_xml__compiled_version(void) { @@ -361,6 +369,28 @@ (*svn_parser->data_handler)(svn_parser->baton, s, (apr_size_t)len); } +#if XML_VERSION_AT_LEAST(1, 95, 8) +static void expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + svn_xml_parser_t *svn_parser = userData; + + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(svn_parser->parser, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif /*** Making a parser. ***/ @@ -382,6 +412,12 @@ XML_SetCharacterDataHandler(parser, data_handler ? expat_data_handler : NULL); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(parser, expat_entity_declaration); +#else + XML_SetDefaultHandler(parser, expat_default_handler); +#endif + /* ### we probably don't want this pool; or at least we should pass it ### to the callbacks and clear it periodically. */ subpool = svn_pool_create(pool); @@ -463,6 +499,9 @@ /* This will cause the current XML_Parse() call to finish quickly! */ XML_SetElementHandler(svn_parser->parser, NULL, NULL); XML_SetCharacterDataHandler(svn_parser->parser, NULL); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(svn_parser->parser, NULL); +#endif /* Once outside of XML_Parse(), the existence of this field will cause svn_delta_parse()'s main read-loop to return error. */ Index: tools/server-side/mod_dontdothat/mod_dontdothat.c =================================================================== --- tools/server-side/mod_dontdothat/mod_dontdothat.c (revision 1768981) +++ tools/server-side/mod_dontdothat/mod_dontdothat.c (working copy) @@ -42,6 +42,14 @@ extern module AP_MODULE_DECLARE_DATA dontdothat_module; +#ifndef XML_VERSION_AT_LEAST +#define XML_VERSION_AT_LEAST(major,minor,patch) \ +(((major) < XML_MAJOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) < XML_MINOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) == XML_MINOR_VERSION && \ + (patch) <= XML_MICRO_VERSION)) +#endif /* XML_VERSION_AT_LEAST */ + typedef struct dontdothat_config_rec { const char *config_file; const char *base_path; @@ -551,6 +559,31 @@ } } +#if XML_VERSION_AT_LEAST(1, 95, 8) +static void +expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + dontdothat_filter_ctx *ctx = userData; + + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(ctx->xmlp, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void +expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif + static svn_boolean_t is_valid_wildcard(const char *wc) { @@ -696,6 +729,12 @@ XML_SetElementHandler(ctx->xmlp, start_element, end_element); XML_SetCharacterDataHandler(ctx->xmlp, cdata); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(ctx->xmlp, expat_entity_declaration); +#else + XML_SetDefaultHandler(ctx->xmlp, expat_default_handler); +#endif + ap_add_input_filter("DONTDOTHAT_FILTER", ctx, r, r->connection); } } ]]] Patch for Subversion 1.8.16: [[[ Index: subversion/libsvn_ra_serf/util.c =================================================================== --- subversion/libsvn_ra_serf/util.c (revision 1768982) +++ subversion/libsvn_ra_serf/util.c (working copy) @@ -2694,7 +2694,31 @@ #endif } +#ifdef EXPAT_HAS_STOPPARSER +static void +expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + struct expat_ctx_t *ectx = userData; + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(ectx->parser, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void +expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif + /* Implements svn_ra_serf__response_handler_t */ static svn_error_t * expat_response_handler(serf_request_t *request, @@ -2712,6 +2736,12 @@ XML_SetUserData(ectx->parser, ectx); XML_SetElementHandler(ectx->parser, expat_start, expat_end); XML_SetCharacterDataHandler(ectx->parser, expat_cdata); + +#ifdef EXPAT_HAS_STOPPARSER + XML_SetEntityDeclHandler(ectx->parser, expat_entity_declaration); +#else + XML_SetDefaultHandler(ectx->parser, expat_default_handler); +#endif } /* ### TODO: sline.code < 200 should really be handled by the core */ Index: subversion/libsvn_subr/xml.c =================================================================== --- subversion/libsvn_subr/xml.c (revision 1768982) +++ subversion/libsvn_subr/xml.c (working copy) @@ -259,6 +259,14 @@ } +#ifndef XML_VERSION_AT_LEAST +#define XML_VERSION_AT_LEAST(major,minor,patch) \ +(((major) < XML_MAJOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) < XML_MINOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) == XML_MINOR_VERSION && \ + (patch) <= XML_MICRO_VERSION)) +#endif /* XML_VERSION_AT_LEAST */ + const char * svn_xml_fuzzy_escape(const char *string, apr_pool_t *pool) { @@ -338,6 +346,28 @@ (*svn_parser->data_handler)(svn_parser->baton, s, (apr_size_t)len); } +#if XML_VERSION_AT_LEAST(1, 95, 8) +static void expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + svn_xml_parser_t *svn_parser = userData; + + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(svn_parser->parser, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif /*** Making a parser. ***/ @@ -359,6 +389,12 @@ XML_SetCharacterDataHandler(parser, data_handler ? expat_data_handler : NULL); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(parser, expat_entity_declaration); +#else + XML_SetDefaultHandler(parser, expat_default_handler); +#endif + /* ### we probably don't want this pool; or at least we should pass it ### to the callbacks and clear it periodically. */ subpool = svn_pool_create(pool); @@ -440,6 +476,9 @@ /* This will cause the current XML_Parse() call to finish quickly! */ XML_SetElementHandler(svn_parser->parser, NULL, NULL); XML_SetCharacterDataHandler(svn_parser->parser, NULL); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(svn_parser->parser, NULL); +#endif /* Once outside of XML_Parse(), the existence of this field will cause svn_delta_parse()'s main read-loop to return error. */ Index: tools/server-side/mod_dontdothat/mod_dontdothat.c =================================================================== --- tools/server-side/mod_dontdothat/mod_dontdothat.c (revision 1768982) +++ tools/server-side/mod_dontdothat/mod_dontdothat.c (working copy) @@ -42,6 +42,14 @@ module AP_MODULE_DECLARE_DATA dontdothat_module; +#ifndef XML_VERSION_AT_LEAST +#define XML_VERSION_AT_LEAST(major,minor,patch) \ +(((major) < XML_MAJOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) < XML_MINOR_VERSION) \ + || ((major) == XML_MAJOR_VERSION && (minor) == XML_MINOR_VERSION && \ + (patch) <= XML_MICRO_VERSION)) +#endif /* XML_VERSION_AT_LEAST */ + typedef struct dontdothat_config_rec { const char *config_file; const char *base_path; @@ -551,6 +559,31 @@ } } +#if XML_VERSION_AT_LEAST(1, 95, 8) +static void +expat_entity_declaration(void *userData, + const XML_Char *entityName, + int is_parameter_entity, + const XML_Char *value, + int value_length, + const XML_Char *base, + const XML_Char *systemId, + const XML_Char *publicId, + const XML_Char *notationName) +{ + dontdothat_filter_ctx *ctx = userData; + + /* Stop the parser if an entity declaration is hit. */ + XML_StopParser(ctx->xmlp, 0 /* resumable */); +} +#else +/* A noop default_handler. */ +static void +expat_default_handler(void *userData, const XML_Char *s, int len) +{ +} +#endif + static svn_boolean_t is_valid_wildcard(const char *wc) { @@ -696,6 +729,12 @@ XML_SetElementHandler(ctx->xmlp, start_element, end_element); XML_SetCharacterDataHandler(ctx->xmlp, cdata); +#if XML_VERSION_AT_LEAST(1, 95, 8) + XML_SetEntityDeclHandler(ctx->xmlp, expat_entity_declaration); +#else + XML_SetDefaultHandler(ctx->xmlp, expat_default_handler); +#endif + ap_add_input_filter("DONTDOTHAT_FILTER", ctx, r, r->connection); } } ]]]