Re: [xml] Redhat security update for libxml2

On Wed, Nov 19, 2008 at 10:31:05AM +0100, Mike Hommey wrote:
On Wed, Nov 19, 2008 at 09:28:30AM +0100, Daniel Veillard <veillard redhat com> wrote:
On Tue, Nov 18, 2008 at 08:28:49PM +0100, Mike Hommey wrote:
On Tue, Nov 18, 2008 at 07:16:50PM +0000, Graham Bennett wrote:
Hi all,

I've been notified of a Redhat security update for libxml2:, and was hoping to
update my own builds with a version that doesn't suffer from these
vulnerabilities (I build from the standard source distribution, not the
Redhat source).  

It wasn't immediately obvious from the release notes and recent mailing
list traffic if these have been fixed in a released version of the
libxml distribution yet.  If they haven't, is a new released planned to
address them?

  Yeah sorry about that. Basically it was embargoed until monday, it's
not that easy to trigger the bugs, I didn't generate a new release for
this I will probably do one within a week or so including those and I
hope a solution for the PHP SAX problem.

Speaking of which, the patch for the SAX2Characters issue seems strange
to me. While it is okay on 32-bits architectures, it doesn't make much
sense on 64-bits architectures, where the addition of 2 ints can hardly
be greater than SIZE_T_MAX.
FWIW, as SIZE_T_MAX was not defined on glibc, the patch I applied on
debian replaces SIZE_T_MAX with UINT_MAX.

  Actually in SVN there is a define of SIZE_T_MAX as (size_t) -1 which
solves the pxprotability problem.

ctxt->nodelen, ctxt->nodemem, and len are all ints.
On 64 bits arches, ctxt->nodelen won't ever be greater than
SIZE_T_MAX - len, because 2^31 won't ever be greater than 2^64 - 2^31.
Likewise, ctxt->nodemem + len won't ever be greater than SIZE_T_MAX / 2
because 2^32 won't ever be greater than 2^63.

gcc might even remove the if clause, actually...

So, without this overflow prevention code doing something, we're left
with a ctxt->nodemem + len that still can overflow in the following if

Now, writing that, I realize my own code, with UINT_MAX, might not prevent
much either, because I don't cast to unsigned int...

And actually, it's even worse than that, because the following size *= 2
overflows before ctxt->nodemem + len does. And changing size type to
size_t only defers the overflow to later, when it is assigned to
ctxt->nodemem a few lines after.

The testcase actually fails in xmlRealloc right after the size *= 2
overflow before hitting the original overflow prevention code. It
obviously also does so without the patch.

I wasn't able to trigger the overflow prevention code on x86 nor x86-64
with the patches from svn nor with some more changes as suggested
earlier. My guess is this will only trigger for some particular values
of MINLEN in xmlIO.c, or with custom IO functions.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]