Patchwork PR other/54411: libiberty: objalloc_alloc integer overflows (CVE-2012-3509)

login
register
mail settings
Submitter Florian Weimer
Date Aug. 29, 2012, 5:32 p.m.
Message ID <876281zjr0.fsf@mid.deneb.enyo.de>
Download mbox | patch
Permalink /patch/180737/
State New
Headers show

Comments

Florian Weimer - Aug. 29, 2012, 5:32 p.m.
This patches fixes an integer overflow in libiberty, which leads to
crashes in binutils.  The long version of the objalloc_alloc macro
would have needed another conditional, so I removed that and replaced
it with a call to the actual implementation.

This has been compiled-tested only.  We do not use this function in
GCC, therefore I want to commit this just to the trunk.

2012-08-29  Florian Weimer  <fw@deneb.enyo.de>

	PR other/54411
	* objalloc.h (objalloc_alloc): Always use the simple definition of
	the macro.

2012-08-29  Florian Weimer  <fw@deneb.enyo.de>

	PR other/54411
	* objalloc.c (_objalloc_alloc): Add overflow check covering
	alignment and CHUNK_HEADER_SIZE addition.
Ian Taylor - Sept. 5, 2012, 5:31 a.m.
On Wed, Aug 29, 2012 at 10:32 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>
> This patches fixes an integer overflow in libiberty, which leads to
> crashes in binutils.  The long version of the objalloc_alloc macro
> would have needed another conditional, so I removed that and replaced
> it with a call to the actual implementation.

I guess I don't see why removing the macro is desirable.  In many uses
of objalloc_alloc the conditional can be optimized out anyhow.  It's
true that it can't always be, but, so what?  The macro is probably
still a win.

Or perhaps the macro is not a win, but that would be a decision to
make separately from adding this conditional.

Ian

Patch

Index: include/objalloc.h
===================================================================
--- include/objalloc.h	(revision 190780)
+++ include/objalloc.h	(working copy)
@@ -1,5 +1,5 @@ 
 /* objalloc.h -- routines to allocate memory for objects
-   Copyright 1997, 2001 Free Software Foundation, Inc.
+   Copyright 1997-2012 Free Software Foundation, Inc.
    Written by Ian Lance Taylor, Cygnus Solutions.
 
 This program is free software; you can redistribute it and/or modify it
@@ -71,38 +71,8 @@ 
 
 extern void *_objalloc_alloc (struct objalloc *, unsigned long);
 
-/* The macro version of objalloc_alloc.  We only define this if using
-   gcc, because otherwise we would have to evaluate the arguments
-   multiple times, or use a temporary field as obstack.h does.  */
-
-#if defined (__GNUC__) && defined (__STDC__) && __STDC__
-
-/* NextStep 2.0 cc is really gcc 1.93 but it defines __GNUC__ = 2 and
-   does not implement __extension__.  But that compiler doesn't define
-   __GNUC_MINOR__.  */
-#if __GNUC__ < 2 || (__NeXT__ && !__GNUC_MINOR__)
-#define __extension__
-#endif
-
-#define objalloc_alloc(o, l)						\
-  __extension__								\
-  ({ struct objalloc *__o = (o);					\
-     unsigned long __len = (l);						\
-     if (__len == 0)							\
-       __len = 1;							\
-     __len = (__len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1);	\
-     (__len <= __o->current_space					\
-      ? (__o->current_ptr += __len,					\
-	 __o->current_space -= __len,					\
-	 (void *) (__o->current_ptr - __len))				\
-      : _objalloc_alloc (__o, __len)); })
-
-#else /* ! __GNUC__ */
-
 #define objalloc_alloc(o, l) _objalloc_alloc ((o), (l))
 
-#endif /* ! __GNUC__ */
-
 /* Free an entire objalloc structure.  */
 
 extern void objalloc_free (struct objalloc *);
Index: libiberty/objalloc.c
===================================================================
--- libiberty/objalloc.c	(revision 190780)
+++ libiberty/objalloc.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* objalloc.c -- routines to allocate memory for objects
-   Copyright 1997 Free Software Foundation, Inc.
+   Copyright 1997-2012 Free Software Foundation, Inc.
    Written by Ian Lance Taylor, Cygnus Solutions.
 
 This program is free software; you can redistribute it and/or modify it
@@ -112,8 +112,9 @@ 
 /* Allocate space from an objalloc structure.  */
 
 PTR
-_objalloc_alloc (struct objalloc *o, unsigned long len)
+_objalloc_alloc (struct objalloc *o, unsigned long original_len)
 {
+  unsigned long len = original_len;
   /* We avoid confusion from zero sized objects by always allocating
      at least 1 byte.  */
   if (len == 0)
@@ -121,6 +122,11 @@ 
 
   len = (len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1);
 
+  /* Check for overflow in the alignment operator above and the malloc
+     argument below. */
+  if (len + CHUNK_HEADER_SIZE < original_len)
+    return NULL;
+
   if (len <= o->current_space)
     {
       o->current_ptr += len;