From 63ad0bd8236a9f0a1dec894af8f7826ae36ac93c Mon Sep 17 00:00:00 2001
From: Evgeny Kotkov <kotkov@apache.org>
Date: Mon, 21 Nov 2016 13:04:14 +0000
Subject: [PATCH] Merge r1770677 from trunk:

 * r1770677
   Make the XML parser stricter by prohibiting custom XML entities.
   Justification:
     The parser should properly error out on invalid data.
   Votes:
     +1: kotkov, rhuijben, stefan2


git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/1.8.x@1770680 13f79535-47bb-0310-9956-ffa450edef68
---
 subversion/libsvn_ra_serf/util.c                  | 30 +++++++++++++++++
 subversion/libsvn_subr/xml.c                      | 39 +++++++++++++++++++++++
 contrib/server-side/mod_dontdothat/mod_dontdothat.c | 39 +++++++++++++++++++++++
 3 files changed, 108 insertions(+)

Index: subversion-1.6.17/subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion-1.6.17.orig/subversion/libsvn_ra_serf/util.c
+++ subversion-1.6.17/subversion/libsvn_ra_serf/util.c
@@ -590,6 +590,30 @@ svn_ra_serf__context_run_wait(svn_boolea
   return SVN_NO_ERROR;
 }
 
+#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
 
 /*
  * Expat callback invoked on a start element tag for an error response.
@@ -1039,6 +1063,11 @@ svn_ra_serf__handle_xml_parser(serf_requ
         {
           XML_SetCharacterDataHandler(ctx->xmlp, cdata_xml);
         }
+#ifdef EXPAT_HAS_STOPPARSER
+      XML_SetEntityDeclHandler(ctx->xmlp, expat_entity_declaration);
+#else
+      XML_SetDefaultHandler(ctx->xmlp, expat_default_handler);
+#endif
     }
 
   while (1)
Index: subversion-1.6.17/subversion/libsvn_subr/xml.c
===================================================================
--- subversion-1.6.17.orig/subversion/libsvn_subr/xml.c
+++ subversion-1.6.17/subversion/libsvn_subr/xml.c
@@ -253,6 +253,14 @@ svn_xml_escape_attr_cstring(svn_stringbu
 }
 
 
+#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)
 {
@@ -331,6 +339,28 @@ static void expat_data_handler(void *use
   (*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. ***/
 
@@ -352,6 +382,12 @@ svn_xml_make_parser(void *baton,
   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);
@@ -433,6 +469,9 @@ void svn_xml_signal_bailout(svn_error_t
   /* 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: subversion-1.6.17/contrib/server-side/mod_dontdothat/mod_dontdothat.c
===================================================================
--- subversion-1.6.17.orig/contrib/server-side/mod_dontdothat/mod_dontdothat.c
+++ subversion-1.6.17/contrib/server-side/mod_dontdothat/mod_dontdothat.c
@@ -37,6 +37,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 {
   const char *config_file;
   const char *base_path;
@@ -546,6 +554,31 @@ end_element(void *baton, const char *nam
     }
 }
 
+#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)
 {
@@ -690,6 +723,12 @@ dontdothat_insert_filters(request_rec *r
       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);
     }
 }
