From 92f8fd757548ff775f1f350907e649d4ea04d77c Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 6 May 2019 13:43:15 +0200 Subject: [Backport] Fix for security issue 894933 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [libxml] Switched from unsigned long to ptrdiff_t in parser.c Using unsigned long instead of ptrdiff_t results in non-zero pointer deltas being stored as zero delta, giving incorrect offsets into arrays and hence out of bounds reads. This patch fixes the issue in all places in parser.c and adds a macro to reduce the chances of cut-and-paste errors. A chromium patch for the libxml roll is added to apply the change to future versions. The issue has been reported upstream to libxml2 because there are likely other places in the code where this fails. R=dcheng@chromium.org,palmer@chromium.org BUG=894933 Change-Id: I13687c386231374653e499e0557838ed0ddea6af Reviewed-on: https://chromium-review.googlesource.com/c/1483252 Auto-Submit: Stephen Chenney Commit-Queue: Daniel Cheng Reviewed-by: Daniel Cheng Cr-Commit-Position: refs/heads/master@{#634859} Reviewed-by: Michael Brüning --- chromium/third_party/libxml/README.chromium | 4 +- chromium/third_party/libxml/src/parser.c | 60 +++++++++-------------------- 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/chromium/third_party/libxml/README.chromium b/chromium/third_party/libxml/README.chromium index c77276cb96e..ac875c0488c 100644 --- a/chromium/third_party/libxml/README.chromium +++ b/chromium/third_party/libxml/README.chromium @@ -15,8 +15,10 @@ Modifications: - Applied these patches in chromium/*.patch: chromium-issue-599427.patch: workaround for VS 2015 Update 2 code-gen bug chromium-issue-628581.patch: See https://crbug.com/628581#c18 + chromium-issue-894933.patch: Use ptrdiff_t instead of unsigned long for + pointer differences in parser.c libxml2-2.9.4-security-CVE-2017-7376-nanohttp-out-of-bounds-write.patch: - See https://crbug.com/708433 + See https://crbug.com/708433 libxml2-2.9.4-security-xpath-nodetab-uaf.patch: See https://crbug.com/705445 chromium-issue-708434.patch: Guard against input counter overflow. - Delete various unused files, see chromium/roll.py diff --git a/chromium/third_party/libxml/src/parser.c b/chromium/third_party/libxml/src/parser.c index 3a8a0d79e96..4e767091683 100644 --- a/chromium/third_party/libxml/src/parser.c +++ b/chromium/third_party/libxml/src/parser.c @@ -2081,8 +2081,8 @@ static void xmlSHRINK (xmlParserCtxtPtr ctxt) { xmlGROW (ctxt); static void xmlGROW (xmlParserCtxtPtr ctxt) { - unsigned long curEnd = ctxt->input->end - ctxt->input->cur; - unsigned long curBase = ctxt->input->cur - ctxt->input->base; + ptrdiff_t curEnd = ctxt->input->end - ctxt->input->cur; + ptrdiff_t curBase = ctxt->input->cur - ctxt->input->base; if (((curEnd > (unsigned long) XML_MAX_LOOKUP_LIMIT) || (curBase > (unsigned long) XML_MAX_LOOKUP_LIMIT)) && @@ -8857,6 +8857,18 @@ xmlParseQNameAndCompare(xmlParserCtxtPtr ctxt, xmlChar const *name, * caller if it was copied, this can be detected by val[*len] == 0. */ +#define GROW_PARSE_ATT_VALUE_INTERNAL(ctxt, in, start, end) \ + const xmlChar *oldbase = ctxt->input->base;\ + GROW;\ + if (ctxt->instate == XML_PARSER_EOF)\ + return(NULL);\ + if (oldbase != ctxt->input->base) {\ + ptrdiff_t delta = ctxt->input->base - oldbase;\ + start = start + delta;\ + in = in + delta;\ + }\ + end = ctxt->input->end; + static xmlChar * xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, int normalize) @@ -8886,14 +8898,7 @@ xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, end = ctxt->input->end; start = in; if (in >= end) { - const xmlChar *oldbase = ctxt->input->base; - GROW; - if (oldbase != ctxt->input->base) { - long delta = ctxt->input->base - oldbase; - start = start + delta; - in = in + delta; - } - end = ctxt->input->end; + GROW_PARSE_ATT_VALUE_INTERNAL(ctxt, in, start, end) } if (normalize) { /* @@ -8910,16 +8915,7 @@ xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, in++; start = in; if (in >= end) { - const xmlChar *oldbase = ctxt->input->base; - GROW; - if (ctxt->instate == XML_PARSER_EOF) - return(NULL); - if (oldbase != ctxt->input->base) { - long delta = ctxt->input->base - oldbase; - start = start + delta; - in = in + delta; - } - end = ctxt->input->end; + GROW_PARSE_ATT_VALUE_INTERNAL(ctxt, in, start, end) if (((in - start) > XML_MAX_TEXT_LENGTH) && ((ctxt->options & XML_PARSE_HUGE) == 0)) { xmlFatalErrMsg(ctxt, XML_ERR_ATTRIBUTE_NOT_FINISHED, @@ -8933,16 +8929,7 @@ xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, col++; if ((*in++ == 0x20) && (*in == 0x20)) break; if (in >= end) { - const xmlChar *oldbase = ctxt->input->base; - GROW; - if (ctxt->instate == XML_PARSER_EOF) - return(NULL); - if (oldbase != ctxt->input->base) { - long delta = ctxt->input->base - oldbase; - start = start + delta; - in = in + delta; - } - end = ctxt->input->end; + GROW_PARSE_ATT_VALUE_INTERNAL(ctxt, in, start, end) if (((in - start) > XML_MAX_TEXT_LENGTH) && ((ctxt->options & XML_PARSE_HUGE) == 0)) { xmlFatalErrMsg(ctxt, XML_ERR_ATTRIBUTE_NOT_FINISHED, @@ -8971,7 +8958,7 @@ xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, if (ctxt->instate == XML_PARSER_EOF) return(NULL); if (oldbase != ctxt->input->base) { - long delta = ctxt->input->base - oldbase; + ptrdiff_t delta = ctxt->input->base - oldbase; start = start + delta; in = in + delta; last = last + delta; @@ -8998,16 +8985,7 @@ xmlParseAttValueInternal(xmlParserCtxtPtr ctxt, int *len, int *alloc, in++; col++; if (in >= end) { - const xmlChar *oldbase = ctxt->input->base; - GROW; - if (ctxt->instate == XML_PARSER_EOF) - return(NULL); - if (oldbase != ctxt->input->base) { - long delta = ctxt->input->base - oldbase; - start = start + delta; - in = in + delta; - } - end = ctxt->input->end; + GROW_PARSE_ATT_VALUE_INTERNAL(ctxt, in, start, end) if (((in - start) > XML_MAX_TEXT_LENGTH) && ((ctxt->options & XML_PARSE_HUGE) == 0)) { xmlFatalErrMsg(ctxt, XML_ERR_ATTRIBUTE_NOT_FINISHED, -- cgit v1.2.1