From a76e6535dc91d65de27f194861a5aa21e9b26365 Mon Sep 17 00:00:00 2001 From: Chong Yidong Date: Tue, 3 Jul 2012 13:28:42 +0800 Subject: [PATCH] * xml.el: Protect parser against XML bombs. (xml-entity-expansion-limit): New variable. (xml-parse-string, xml-substitute-special): Use it. (xml-parse-dtd): Avoid infloop if the DTD is not terminated. * test/automated/xml-parse-tests.el: Update testcases. --- lisp/ChangeLog | 7 +++++++ lisp/xml.el | 37 +++++++++++++++++++++++++++++++------ test/ChangeLog | 4 ++++ test/automated/xml-parse-tests.el | 19 +++++++++++++++++-- 4 files changed, 59 insertions(+), 8 deletions(-) diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 58cb7e69688..dd136f5b053 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,10 @@ +2012-07-03 Chong Yidong + + * xml.el: Protect parser against XML bombs. + (xml-entity-expansion-limit): New variable. + (xml-parse-string, xml-substitute-special): Use it. + (xml-parse-dtd): Avoid infloop if the DTD is not terminated. + 2012-07-03 Glenn Morris * progmodes/bug-reference.el (bug-reference-bug-regexp): diff --git a/lisp/xml.el b/lisp/xml.el index a3e279b41bd..2595fd572f4 100644 --- a/lisp/xml.el +++ b/lisp/xml.el @@ -98,6 +98,13 @@ ("amp" . "&")) "Alist mapping XML entities to their replacement text.") +(defvar xml-entity-expansion-limit 20000 + "The maximum size of entity reference expansions. +If the size of the buffer increases by this many characters while +expanding entity references in a segment of character data, the +XML parser signals an error. Setting this to nil removes the +limit (making the parser vulnerable to XML bombs).") + (defvar xml-parameter-entity-alist nil "Alist of defined XML parametric entities.") @@ -471,7 +478,7 @@ Return one of: (while (not (looking-at end)) (cond ((eobp) - (error "XML: (Not Well-Formed) End of buffer while reading element `%s'" + (error "XML: (Not Well-Formed) End of document while reading element `%s'" node-name)) ((looking-at " (- (buffer-size) (point)) + (+ old-remaining-size xml-entity-expansion-limit)) + (error "XML: Entity reference expansion \ +surpassed `xml-entity-expansion-limit'")))) ;; [2.11] Clean up line breaks. (let ((end-marker (point-marker))) (goto-char start) @@ -689,6 +704,8 @@ This follows the rule [28] in the XML specifications." (while (not (looking-at "\\s-*\\]")) (skip-syntax-forward " ") (cond + ((eobp) + (error "XML: (Well-Formed) End of document while reading DTD")) ;; Element declaration [45]: ((and (looking-at (eval-when-compile (concat "") (goto-char (match-end 0)))) @@ -876,6 +895,7 @@ STRING is assumed to occur in an XML attribute value." (let ((ref-re (eval-when-compile (concat "&\\(?:#\\(x\\)?\\([0-9]+\\)\\|\\(" xml-name-re "\\)\\);"))) + (strlen (length string)) children) (while (string-match ref-re string) (push (substring string 0 (match-beginning 0)) children) @@ -891,7 +911,8 @@ STRING is assumed to occur in an XML attribute value." (error "XML: (Validity) Undefined character `x%s'" ref)) (t xml-undefined-entity)) children) - (setq string remainder)) + (setq string remainder + strlen (length string))) ;; [4.4.5] Entity references are "included in literal". ;; Note that we don't need do anything special to treat ;; quotes as normal data characters. @@ -900,7 +921,11 @@ STRING is assumed to occur in an XML attribute value." (if xml-validating-parser (error "XML: (Validity) Undefined entity `%s'" ref) xml-undefined-entity)))) - (setq string (concat val remainder)))))) + (setq string (concat val remainder))) + (and xml-entity-expansion-limit + (> (length string) (+ strlen xml-entity-expansion-limit)) + (error "XML: Passed `xml-entity-expansion-limit' while expanding `&%s;'" + ref))))) (mapconcat 'identity (nreverse (cons string children)) ""))) (defun xml-substitute-numeric-entities (string) diff --git a/test/ChangeLog b/test/ChangeLog index 3ff7124893a..1e77f972965 100644 --- a/test/ChangeLog +++ b/test/ChangeLog @@ -1,3 +1,7 @@ +2012-07-03 Chong Yidong + + * automated/xml-parse-tests.el (xml-parse-tests--bad-data): New. + 2012-07-02 Chong Yidong * automated/xml-parse-tests.el (xml-parse-tests--data): More diff --git a/test/automated/xml-parse-tests.el b/test/automated/xml-parse-tests.el index ec3d7ca3065..ada9bbd4074 100644 --- a/test/automated/xml-parse-tests.el +++ b/test/automated/xml-parse-tests.el @@ -55,14 +55,29 @@ ("&" . ((foo () "&")))) "Alist of XML strings and their expected parse trees.") +(defvar xml-parse-tests--bad-data + '(;; XML bomb in content + "]>&lol2;" + ;; XML bomb in attribute value + "]>!" + ;; Non-terminating DTD + "" + "asdf" + "asdf&abc;") + "List of XML strings that should signal an error in the parser") + (ert-deftest xml-parse-tests () "Test XML parsing." (with-temp-buffer (dolist (test xml-parse-tests--data) (erase-buffer) (insert (car test)) - (should (equal (cdr test) - (xml-parse-region (point-min) (point-max))))))) + (should (equal (cdr test) (xml-parse-region)))) + (let ((xml-entity-expansion-limit 50)) + (dolist (test xml-parse-tests--bad-data) + (erase-buffer) + (insert test) + (should-error (xml-parse-region)))))) ;; Local Variables: ;; no-byte-compile: t -- 2.11.4.GIT