Re: [xml] Redhat security update for libxml2

[Mike Hommey <mh glandium org>]

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:
> > > https://rhn.redhat.com/errata/RHSA-2008-0988.html, 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
clause.

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

Mike