Thread: Another XML build issue

Another XML build issue

From
Kevin Grittner
Date:
Today, on my ubuntu 14.04 LTS system, I saw an XML change come through
with updates.  My build on master is now broken with this:

*** /home/kgrittn/pg/master/src/test/regress/expected/xml.out
2015-11-23 08:15:28.206336200 -0600
--- /home/kgrittn/pg/master/src/test/regress/results/xml.out
2015-12-14 09:29:40.489533194 -0600
***************
*** 271,279 **** line 1: Document is empty
 ^
- line 1: Start tag expected, '<' not found
-
- ^ SELECT xmlparse(document '   '); ERROR:  invalid XML document DETAIL:  line 1: Start tag expected, '<' not found
--- 271,276 ----

======================================================================

Any ideas on what to do about this one?

I could certainly push yet another version of the expected file, but
I'm not sure that's the thing to do.  I don't see it in the buildfarm
yet, but that's probably just a matter of time.

-- 
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Another XML build issue

From
Tom Lane
Date:
Kevin Grittner <kgrittn@gmail.com> writes:
> Today, on my ubuntu 14.04 LTS system, I saw an XML change come through
> with updates.  My build on master is now broken with this:

> *** /home/kgrittn/pg/master/src/test/regress/expected/xml.out
> 2015-11-23 08:15:28.206336200 -0600
> --- /home/kgrittn/pg/master/src/test/regress/results/xml.out
> 2015-12-14 09:29:40.489533194 -0600
> ***************
> *** 271,279 ****
>   line 1: Document is empty

>   ^
> - line 1: Start tag expected, '<' not found
> -
> - ^
>   SELECT xmlparse(document '   ');
>   ERROR:  invalid XML document
>   DETAIL:  line 1: Start tag expected, '<' not found
> --- 271,276 ----

That is odd; I did not think there'd be more than one new behavior
from libxml2.  Can you look to see if Ubuntu is carrying some
distro-specific patch that affects this?
        regards, tom lane



Re: Another XML build issue

From
Kevin Grittner
Date:
On Mon, Dec 14, 2015 at 9:44 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Can you look to see if Ubuntu is carrying some
> distro-specific patch that affects this?

Here's what is in the log for the change that I think is the one that
came through today:

============================================================

libxml2 (2.9.1+dfsg1-3ubuntu4.6) trusty-security; urgency=medium
 * SECURITY UPDATE: denial of service via entity expansion issue   - debian/patches/CVE-2015-5312.patch: properly exit
whenentity     expansion is detected in parser.c.   - CVE-2015-5312 * SECURITY UPDATE: heap buffer overflow in
xmlDictComputeFastQKey  - debian/patches/CVE-2015-7497.patch: check offset in dict.c.   - CVE-2015-7497 * SECURITY
UPDATE:denial of service via encoding conversion failures   - debian/patches/CVE-2015-7498.patch: avoid processing
entitiesafter     encoding conversion failures in parser.c.   - CVE-2015-7498 * SECURITY UPDATE: out of bounds read in
xmlGROW  - debian/patches/CVE-2015-7499-1.patch: add xmlHaltParser() to stop the     parser in parser.c.   -
debian/patches/CVE-2015-7499-2.patch:check input in parser.c.   - CVE-2015-7499 * SECURITY UPDATE: out of bounds read
inxmlParseMisc   - debian/patches/CVE-2015-7500.patch: check entity boundaries in     parser.c.   - CVE-2015-7500 *
SECURITYUPDATE: denial of service via extra processing of MarkupDecl   - debian/patches/CVE-2015-8241.patch: add extra
EOFcheck in parser.c.   - CVE-2015-8241 * SECURITY UPDATE: buffer overead with HTML parser in push mode   -
debian/patches/CVE-2015-8242.patch:use pointer in the input in     HTMLparser.c.   - CVE-2015-8242 * SECURITY UPDATE:
denialof service via encoding failures   - debian/patches/CVE-2015-8317-1.patch: do not process encoding values     if
thedeclaration is broken in parser.c.   - debian/patches/CVE-2015-8317-2.patch: fail parsing if the encoding
conversionfailed in parser.c.   - CVE-2015-8317
 
-- Marc Deslauriers <marc.deslauriers@ubuntu.com>  Wed, 09 Dec 2015
12:00:30 -0500

============================================================

I don't know how that compares to other distros...

-- 
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Another XML build issue

From
Tom Lane
Date:
Kevin Grittner <kgrittn@gmail.com> writes:
> Today, on my ubuntu 14.04 LTS system, I saw an XML change come through
> with updates.  My build on master is now broken with this:

> - line 1: Start tag expected, '<' not found
> -
> - ^

Now that I look more closely, there are really two distinct pathologies
exhibited in xml_2.out.  One is what I said in the commit message, that
error position reports occurring at EOF are lost.  But the other is
that this specific error message disappears altogether.  Looking at the
larger context:

SELECT xmlparse(document '');
ERROR:  invalid XML document
DETAIL:  line 1: switching encoding : no input

^
line 1: Document is empty

^
line 1: Start tag expected, '<' not found

^

the missing message is one that should have come out after the "document
is empty" message.  I am suspicious that what this is is an additional
side effect of the CVE-2015-7499 fix, which as I understood it was a hack
to prevent further parsing once any error has been detected.

So at this point I'm guessing that Ubuntu has shipped an update that
includes the patch Pavel Raiskup suggested in

https://bugzilla.redhat.com/show_bug.cgi?id=1286692#c4

which would fix the lack of error cursors at EOF, but it would not affect
any larger change such as stopping after the first detected error.

If you can confirm that, then we'll need to decide what to do about it.
Personally I'd vote for just dropping this specific test case, which
does not seem to have enough value to justify carrying YA expected-file.
        regards, tom lane



Re: Another XML build issue

From
Kevin Grittner
Date:
On Mon, Dec 14, 2015 at 10:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> So at this point I'm guessing that Ubuntu has shipped an update that
> includes the patch Pavel Raiskup suggested in
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1286692#c4
>
> which would fix the lack of error cursors at EOF, but it would not affect
> any larger change such as stopping after the first detected error.
>
> If you can confirm that, then we'll need to decide what to do about it.

I've been looking at this diff, which seems to represent the ubuntu
libxml2 changes of today, and it seems rather different to what I
see in the Red Hat discussion.  In particular, I see no changes to
error.c.  I haven't really figured the cause of the discrepancy
yet, but figured I would pass along the diff link.

http://launchpadlibrarian.net/229976362/libxml2_2.9.1%2Bdfsg1-3ubuntu4.5_2.9.1%2Bdfsg1-3ubuntu4.6.diff.gz

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Another XML build issue

From
Tom Lane
Date:
Kevin Grittner <kgrittn@gmail.com> writes:
> On Mon, Dec 14, 2015 at 10:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So at this point I'm guessing that Ubuntu has shipped an update that
>> includes the patch Pavel Raiskup suggested in
>> 
>> https://bugzilla.redhat.com/show_bug.cgi?id=1286692#c4
>> 
>> which would fix the lack of error cursors at EOF, but it would not affect
>> any larger change such as stopping after the first detected error.
>> 
>> If you can confirm that, then we'll need to decide what to do about it.

> I've been looking at this diff, which seems to represent the ubuntu
> libxml2 changes of today, and it seems rather different to what I
> see in the Red Hat discussion.  In particular, I see no changes to
> error.c.  I haven't really figured the cause of the discrepancy
> yet, but figured I would pass along the diff link.

> http://launchpadlibrarian.net/229976362/libxml2_2.9.1%2Bdfsg1-3ubuntu4.5_2.9.1%2Bdfsg1-3ubuntu4.6.diff.gz

Hmm.  What it looks like is that Ubuntu cherry-picked the libxml2 commit
that added xmlHaltParser(), but *not* the follow-on commit that changed
the behavior of xmlParserPrintFileContextInternal().  So that would
probably explain the behavior you're seeing ... though it also raises
the question of whether they've left themselves vulnerable to a null
pointer dereference in xmlParserPrintFileContextInternal().  It's not
clear to me whether that second commit is properly part of the CVE
response or not.  But it's Veillard's patch, and he included it in
Red Hat's libxml2 security update, so an informed guess would be "yes".

Regardless of that, it now seems likely to me that what you're seeing
is what the output will look like if the no-error-cursor-at-EOF problem
gets fixed, which is what we're hoping will happen.  So we had better
change the regression tests to accept this behavior too.

As I said, my inclination is to remove the SELECT xmlparse(document '')
test case altogether.  The following test SELECT xmlparse(document '   ')
seems like it provides as much useful coverage as we need for that,
and its output doesn't depend on whether libxml2 continues parsing
after the first error.
        regards, tom lane



Re: Another XML build issue

From
Kevin Grittner
Date:
On Mon, Dec 14, 2015 at 10:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> As I said, my inclination is to remove the SELECT xmlparse(document '')
> test case altogether.  The following test SELECT xmlparse(document '   ')
> seems like it provides as much useful coverage as we need for that,
> and its output doesn't depend on whether libxml2 continues parsing
> after the first error.

That's fine with me.  I can do that, if there are no objections.
(It is easy enough to find the affected lines in all 3 "expected"
files.)

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Another XML build issue

From
Kevin Grittner
Date:
On Mon, Dec 14, 2015 at 10:56 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 10:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> As I said, my inclination is to remove the SELECT xmlparse(document '')
>> test case altogether.  The following test SELECT xmlparse(document '   ')
>> seems like it provides as much useful coverage as we need for that,
>> and its output doesn't depend on whether libxml2 continues parsing
>> after the first error.
>
> That's fine with me.  I can do that, if there are no objections.
> (It is easy enough to find the affected lines in all 3 "expected"
> files.)

This didn't seem terribly controversial, so pushed.

-- 
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company