0011-Fix-handling-of-parameter-entity-references.patch 9.4 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280
  1. From: Nick Wellnhofer <wellnhofer@aevum.de>
  2. Date: Mon, 5 Jun 2017 15:37:17 +0200
  3. Subject: Fix handling of parameter-entity references
  4. MIME-Version: 1.0
  5. Content-Type: text/plain; charset=UTF-8
  6. Content-Transfer-Encoding: 8bit
  7. Origin: https://git.gnome.org/browse/libxml2/commit/?id=e26630548e7d138d2c560844c43820b6767251e3
  8. Bug: https://bugzilla.gnome.org/show_bug.cgi?id=781205
  9. Bug-Debian: https://bugs.debian.org/863019
  10. Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-9049
  11. Bug: https://bugzilla.gnome.org/show_bug.cgi?id=781361
  12. Bug-Debian: https://bugs.debian.org/863018
  13. Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-9050
  14. There were two bugs where parameter-entity references could lead to an
  15. unexpected change of the input buffer in xmlParseNameComplex and
  16. xmlDictLookup being called with an invalid pointer.
  17. Percent sign in DTD Names
  18. =========================
  19. The NEXTL macro used to call xmlParserHandlePEReference. When parsing
  20. "complex" names inside the DTD, this could result in entity expansion
  21. which created a new input buffer. The fix is to simply remove the call
  22. to xmlParserHandlePEReference from the NEXTL macro. This is safe because
  23. no users of the macro require expansion of parameter entities.
  24. - xmlParseNameComplex
  25. - xmlParseNCNameComplex
  26. - xmlParseNmtoken
  27. The percent sign is not allowed in names, which are grammatical tokens.
  28. - xmlParseEntityValue
  29. Parameter-entity references in entity values are expanded but this
  30. happens in a separate step in this function.
  31. - xmlParseSystemLiteral
  32. Parameter-entity references are ignored in the system literal.
  33. - xmlParseAttValueComplex
  34. - xmlParseCharDataComplex
  35. - xmlParseCommentComplex
  36. - xmlParsePI
  37. - xmlParseCDSect
  38. Parameter-entity references are ignored outside the DTD.
  39. - xmlLoadEntityContent
  40. This function is only called from xmlStringLenDecodeEntities and
  41. entities are replaced in a separate step immediately after the function
  42. call.
  43. This bug could also be triggered with an internal subset and double
  44. entity expansion.
  45. This fixes bug 766956 initially reported by Wei Lei and independently by
  46. Chromium's ClusterFuzz, Hanno Böck, and Marco Grassi. Thanks to everyone
  47. involved.
  48. xmlParseNameComplex with XML_PARSE_OLD10
  49. ========================================
  50. When parsing Names inside an expanded parameter entity with the
  51. XML_PARSE_OLD10 option, xmlParseNameComplex would call xmlGROW via the
  52. GROW macro if the input buffer was exhausted. At the end of the
  53. parameter entity's replacement text, this function would then call
  54. xmlPopInput which invalidated the input buffer.
  55. There should be no need to invoke GROW in this situation because the
  56. buffer is grown periodically every XML_PARSER_CHUNK_SIZE characters and,
  57. at least for UTF-8, in xmlCurrentChar. This also matches the code path
  58. executed when XML_PARSE_OLD10 is not set.
  59. This fixes bugs 781205 (CVE-2017-9049) and 781361 (CVE-2017-9050).
  60. Thanks to Marcel Böhme and Thuan Pham for the report.
  61. Additional hardening
  62. ====================
  63. A separate check was added in xmlParseNameComplex to validate the
  64. buffer size.
  65. ---
  66. Makefile.am | 18 ++++++++++++++++++
  67. parser.c | 18 ++++++++++--------
  68. result/errors10/781205.xml | 0
  69. result/errors10/781205.xml.err | 21 +++++++++++++++++++++
  70. result/errors10/781361.xml | 0
  71. result/errors10/781361.xml.err | 13 +++++++++++++
  72. result/valid/766956.xml | 0
  73. result/valid/766956.xml.err | 9 +++++++++
  74. result/valid/766956.xml.err.rdr | 10 ++++++++++
  75. runtest.c | 3 +++
  76. test/errors10/781205.xml | 3 +++
  77. test/errors10/781361.xml | 3 +++
  78. test/valid/766956.xml | 2 ++
  79. test/valid/dtds/766956.dtd | 2 ++
  80. 14 files changed, 94 insertions(+), 8 deletions(-)
  81. create mode 100644 result/errors10/781205.xml
  82. create mode 100644 result/errors10/781205.xml.err
  83. create mode 100644 result/errors10/781361.xml
  84. create mode 100644 result/errors10/781361.xml.err
  85. create mode 100644 result/valid/766956.xml
  86. create mode 100644 result/valid/766956.xml.err
  87. create mode 100644 result/valid/766956.xml.err.rdr
  88. create mode 100644 test/errors10/781205.xml
  89. create mode 100644 test/errors10/781361.xml
  90. create mode 100644 test/valid/766956.xml
  91. create mode 100644 test/valid/dtds/766956.dtd
  92. --- a/Makefile.am
  93. +++ b/Makefile.am
  94. @@ -422,6 +422,24 @@ Errtests : xmllint$(EXEEXT)
  95. if [ -n "$$log" ] ; then echo $$name result ; echo $$log ; fi ; \
  96. rm result.$$name error.$$name ; \
  97. fi ; fi ; done)
  98. + @echo "## Error cases regression tests (old 1.0)"
  99. + -@(for i in $(srcdir)/test/errors10/*.xml ; do \
  100. + name=`basename $$i`; \
  101. + if [ ! -d $$i ] ; then \
  102. + if [ ! -f $(srcdir)/result/errors10/$$name ] ; then \
  103. + echo New test file $$name ; \
  104. + $(CHECKER) $(top_builddir)/xmllint --oldxml10 $$i \
  105. + 2> $(srcdir)/result/errors10/$$name.err \
  106. + > $(srcdir)/result/errors10/$$name ; \
  107. + grep "MORY ALLO" .memdump | grep -v "MEMORY ALLOCATED : 0"; \
  108. + else \
  109. + log=`$(CHECKER) $(top_builddir)/xmllint --oldxml10 $$i 2> error.$$name > result.$$name ; \
  110. + grep "MORY ALLO" .memdump | grep -v "MEMORY ALLOCATED : 0"; \
  111. + diff $(srcdir)/result/errors10/$$name result.$$name ; \
  112. + diff $(srcdir)/result/errors10/$$name.err error.$$name` ; \
  113. + if [ -n "$$log" ] ; then echo $$name result ; echo "$$log" ; fi ; \
  114. + rm result.$$name error.$$name ; \
  115. + fi ; fi ; done)
  116. @echo "## Error cases stream regression tests"
  117. -@(for i in $(srcdir)/test/errors/*.xml ; do \
  118. name=`basename $$i`; \
  119. --- a/parser.c
  120. +++ b/parser.c
  121. @@ -2115,7 +2115,6 @@ static void xmlGROW (xmlParserCtxtPtr ct
  122. ctxt->input->line++; ctxt->input->col = 1; \
  123. } else ctxt->input->col++; \
  124. ctxt->input->cur += l; \
  125. - if (*ctxt->input->cur == '%') xmlParserHandlePEReference(ctxt); \
  126. } while (0)
  127. #define CUR_CHAR(l) xmlCurrentChar(ctxt, &l)
  128. @@ -3406,13 +3405,6 @@ xmlParseNameComplex(xmlParserCtxtPtr ctx
  129. len += l;
  130. NEXTL(l);
  131. c = CUR_CHAR(l);
  132. - if (c == 0) {
  133. - count = 0;
  134. - GROW;
  135. - if (ctxt->instate == XML_PARSER_EOF)
  136. - return(NULL);
  137. - c = CUR_CHAR(l);
  138. - }
  139. }
  140. }
  141. if ((len > XML_MAX_NAME_LENGTH) &&
  142. @@ -3420,6 +3412,16 @@ xmlParseNameComplex(xmlParserCtxtPtr ctx
  143. xmlFatalErr(ctxt, XML_ERR_NAME_TOO_LONG, "Name");
  144. return(NULL);
  145. }
  146. + if (ctxt->input->cur - ctxt->input->base < len) {
  147. + /*
  148. + * There were a couple of bugs where PERefs lead to to a change
  149. + * of the buffer. Check the buffer size to avoid passing an invalid
  150. + * pointer to xmlDictLookup.
  151. + */
  152. + xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
  153. + "unexpected change of input buffer");
  154. + return (NULL);
  155. + }
  156. if ((*ctxt->input->cur == '\n') && (ctxt->input->cur[-1] == '\r'))
  157. return(xmlDictLookup(ctxt->dict, ctxt->input->cur - (len + 1), len));
  158. return(xmlDictLookup(ctxt->dict, ctxt->input->cur - len, len));
  159. --- /dev/null
  160. +++ b/result/errors10/781205.xml.err
  161. @@ -0,0 +1,21 @@
  162. +Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
  163. +
  164. + %a;
  165. + ^
  166. +Entity: line 1:
  167. +<:0000
  168. +^
  169. +Entity: line 1: parser error : DOCTYPE improperly terminated
  170. + %a;
  171. + ^
  172. +Entity: line 1:
  173. +<:0000
  174. +^
  175. +namespace error : Failed to parse QName ':0000'
  176. + %a;
  177. + ^
  178. +<:0000
  179. + ^
  180. +./test/errors10/781205.xml:4: parser error : Couldn't find end of Start Tag :0000 line 1
  181. +
  182. +^
  183. --- /dev/null
  184. +++ b/result/errors10/781361.xml.err
  185. @@ -0,0 +1,13 @@
  186. +./test/errors10/781361.xml:4: parser error : xmlParseElementDecl: 'EMPTY', 'ANY' or '(' expected
  187. +
  188. +^
  189. +./test/errors10/781361.xml:4: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
  190. +
  191. +
  192. +^
  193. +./test/errors10/781361.xml:4: parser error : DOCTYPE improperly terminated
  194. +
  195. +^
  196. +./test/errors10/781361.xml:4: parser error : Start tag expected, '<' not found
  197. +
  198. +^
  199. --- /dev/null
  200. +++ b/result/valid/766956.xml.err
  201. @@ -0,0 +1,9 @@
  202. +test/valid/dtds/766956.dtd:2: parser error : PEReference: expecting ';'
  203. +%ä%ent;
  204. + ^
  205. +Entity: line 1: parser error : Content error in the external subset
  206. + %ent;
  207. + ^
  208. +Entity: line 1:
  209. +value
  210. +^
  211. --- /dev/null
  212. +++ b/result/valid/766956.xml.err.rdr
  213. @@ -0,0 +1,10 @@
  214. +test/valid/dtds/766956.dtd:2: parser error : PEReference: expecting ';'
  215. +%ä%ent;
  216. + ^
  217. +Entity: line 1: parser error : Content error in the external subset
  218. + %ent;
  219. + ^
  220. +Entity: line 1:
  221. +value
  222. +^
  223. +./test/valid/766956.xml : failed to parse
  224. --- a/runtest.c
  225. +++ b/runtest.c
  226. @@ -4202,6 +4202,9 @@ testDesc testDescriptions[] = {
  227. { "Error cases regression tests",
  228. errParseTest, "./test/errors/*.xml", "result/errors/", "", ".err",
  229. 0 },
  230. + { "Error cases regression tests (old 1.0)",
  231. + errParseTest, "./test/errors10/*.xml", "result/errors10/", "", ".err",
  232. + XML_PARSE_OLD10 },
  233. #ifdef LIBXML_READER_ENABLED
  234. { "Error cases stream regression tests",
  235. streamParseTest, "./test/errors/*.xml", "result/errors/", NULL, ".str",
  236. --- /dev/null
  237. +++ b/test/errors10/781205.xml
  238. @@ -0,0 +1,3 @@
  239. +<!DOCTYPE D [
  240. + <!ENTITY % a "<:0000">
  241. + %a;
  242. --- /dev/null
  243. +++ b/test/errors10/781361.xml
  244. @@ -0,0 +1,3 @@
  245. +<!DOCTYPE doc [
  246. + <!ENTITY % elem "<!ELEMENT e0000000000">
  247. + %elem;
  248. --- /dev/null
  249. +++ b/test/valid/766956.xml
  250. @@ -0,0 +1,2 @@
  251. +<!DOCTYPE test SYSTEM "dtds/766956.dtd">
  252. +<test/>
  253. --- /dev/null
  254. +++ b/test/valid/dtds/766956.dtd
  255. @@ -0,0 +1,2 @@
  256. +<!ENTITY % ent "value">
  257. +%ä%ent;