Bug 54411 - libiberty: objalloc_alloc integer overflows (CVE-2012-3509)
Summary: libiberty: objalloc_alloc integer overflows (CVE-2012-3509)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Florian Weimer
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-08-29 17:11 UTC by Florian Weimer
Modified: 2017-07-25 03:38 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-08-29 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2012-08-29 17:11:11 UTC
Sang Kil Cha discovered that _objalloc_alloc does not guard the addition of CHUNK_HEADER_SIZE to the length against overflow.  This can cause _objalloc_alloc to return a pointer to a memory region which is smaller than expected.

The pointer alignment arithmetic in the objalloc_alloc macro misses an overflow check as well, with similar consequences.
Comment 1 Florian Weimer 2012-08-29 17:12:02 UTC
I'll come up with a patch for both issues (I don't think there is one yet).
Comment 2 joseph@codesourcery.com 2012-08-29 19:09:55 UTC
You do realise that pretty much nothing in the toolchain checks for 
overflows in calculating allocation sizes?  If you want to fix such 
issues more systematically, the macros in libiberty.h such as XNEWVEC 
would be a good starting point in which to insert overflow checks, but 
there will be loads of other places affected as well.
Comment 3 Florian Weimer 2012-08-29 19:18:58 UTC
(In reply to comment #2)
> You do realise that pretty much nothing in the toolchain checks for 
> overflows in calculating allocation sizes?

bfd_alloc2 (bfd's XNEWVEC variant) contains an overflow check, and this bug defeats it.

Your point about XNEWVEC is appreciated, but this is a different bug. 8-)
Comment 4 joseph@codesourcery.com 2012-08-29 21:29:12 UTC
On Wed, 29 Aug 2012, fw at gcc dot gnu.org wrote:

> > You do realise that pretty much nothing in the toolchain checks for 
> > overflows in calculating allocation sizes?
> 
> bfd_alloc2 (bfd's XNEWVEC variant) contains an overflow check, and this bug
> defeats it.
> 
> Your point about XNEWVEC is appreciated, but this is a different bug. 8-)

My point is that there are probably hundreds of such bugs in the toolchain 
sources (and lots more bugs where BFD and GDB fail to handle invalid 
input); XNEWVEC is just an example of one that's comparatively easy to 
find and fix (but I'm sure plenty more are easy to find for anyone going 
looking for them).
Comment 5 Florian Weimer 2012-09-18 08:34:12 UTC
Author: fw
Date: Tue Sep 18 08:34:05 2012
New Revision: 191413

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191413
Log:
PR other/54411: integer overflow in objalloc_alloc

2012-09-18  Florian Weimer  <fweimer@redhat.com>

	PR other/54411
	* objalloc.h (objalloc_alloc): Do not use fast path on wraparound.

2012-09-18  Florian Weimer  <fweimer@redhat.com>

	PR other/54411
	* objalloc.c (_objalloc_alloc): Add overflow check covering
	alignment and CHUNK_HEADER_SIZE addition.

Modified:
    trunk/include/ChangeLog
    trunk/include/objalloc.h
    trunk/libiberty/ChangeLog
    trunk/libiberty/objalloc.c
Comment 6 Eric Gallager 2017-07-25 03:38:31 UTC
(In reply to Florian Weimer from comment #5)
> Author: fw
> Date: Tue Sep 18 08:34:05 2012
> New Revision: 191413
> 
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191413
> Log:
> PR other/54411: integer overflow in objalloc_alloc
> 
> 2012-09-18  Florian Weimer  <fweimer@redhat.com>
> 
> 	PR other/54411
> 	* objalloc.h (objalloc_alloc): Do not use fast path on wraparound.
> 
> 2012-09-18  Florian Weimer  <fweimer@redhat.com>
> 
> 	PR other/54411
> 	* objalloc.c (_objalloc_alloc): Add overflow check covering
> 	alignment and CHUNK_HEADER_SIZE addition.
> 
> Modified:
>     trunk/include/ChangeLog
>     trunk/include/objalloc.h
>     trunk/libiberty/ChangeLog
>     trunk/libiberty/objalloc.c

Looks like this fixed it.