Bug 21172 - potential integer overflow error in STL heap functions
Summary: potential integer overflow error in STL heap functions
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P2 enhancement
Target Milestone: 4.3.0
Assignee: Paolo Carlini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-04-23 09:13 UTC by pete_a90
Modified: 2007-02-13 00:26 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-04-28 00:11:11


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description pete_a90 2005-04-23 09:13:48 UTC
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/stl_heap.h?rev=1.16&content-type=text/x-cvsweb-markup

Any function that relies on the __adjust_heap functions have a potential integer overflow error in their calculation of __secondChild.
Comment 1 Paolo Carlini 2005-04-26 15:21:40 UTC
Actually, all the callers of __adjust_heap either pass a null second argument,
or a "small" second argument (case of make_heap). Thus, no real risks. Notice
that the function is an internal detail (double underscore prefix) and is not
supposed to be externally used.
Comment 2 pete_a90 2005-04-27 19:09:06 UTC
I know there are no practical risks to this (heap isn't that popular and it's practically impossible to allocate an array that large) but it won't work if the user made a custom iterator with unsigned char as the size_type and signed char as the distance_type on a machine with small chars. The initialization of __secondChild isn't the problem, it's the updating of it in the loop that will cause it.
Comment 3 Paolo Carlini 2005-04-27 19:26:10 UTC
Please, either provide an analysis that the problem really happens, *given the
specific algorithm*, or provide a testcase.
Comment 4 Paolo Carlini 2005-04-28 00:08:17 UTC
Ok, let's reopen the PR as "enhancement": actually, it's easy to produce a
testcase that leads to __secondChild growing beyond __len and the latter
can be equal to the biggest representable _Distance.
Comment 5 Paolo Carlini 2006-04-17 18:35:49 UTC
Working on a fix.
Comment 6 Paolo Carlini 2006-10-18 11:37:54 UTC
A straightforward approach to the problem uses the unsigned type associated with _Distance (via __gnu_cxx::__add_unsigned) to avoid the risk of overflows in __adjust_heap completely. I'm currently looking into the cleanest way to follow this route...
Comment 7 paolo@gcc.gnu.org 2007-02-13 00:25:41 UTC
Subject: Bug 21172

Author: paolo
Date: Tue Feb 13 00:25:30 2007
New Revision: 121875

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121875
Log:
2007-02-12  Paolo Carlini  <pcarlini@suse.de>

	PR libstdc++/21172
	* include/bits/stl_heap.h (__adjust_heap(_RandomAccessIterator,
	_Distance, _Distance, _Tp), __adjust_heap(_RandomAccessIterator,
	_Distance, _Distance, _Tp, _Compare)): Avoid potential integer
	overflow.

	* include/bits/stl_heap.h (__is_heap(_RandomAccessIterator,
	_RandomAccessIterator), __is_heap(_RandomAccessIterator,
	_RandomAccessIterator, _StrictWeakOrdering): Mark inline.
	(make_heap(_RandomAccessIterator, _RandomAccessIterator,
	_Compare)): Do not mark inline.

	* include/bits/stl_heap.h (push_heap(_RandomAccessIterator,
	_RandomAccessIterator), sort_heap(_RandomAccessIterator,
	_RandomAccessIterator)): Uncomment __glibcxx_requires_heap.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/stl_heap.h

Comment 8 Paolo Carlini 2007-02-13 00:26:33 UTC
Fixed.