[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: security-related gcc bug

Some conflicting messages going around here...  This little point rather
bigger than perhaps makes sense, but anyway:

Howard Chu wrote:
>Hallvard B Furuseth wrote:
>>Howard Chu wrote:
>>> 	char buf[MYSIZE];
>>> 	ber_len_t len;		/* length of current buffer content */
>>> 	struct berval *in;	/* passed in, to be moved into buf */
>>> You just test:
>>> 	if ( in->bv_len>  MYSIZE || in->bv_len + len>  MYSIZE )
>>> 		return FAIL;
>> Except that in->bv_len + len can wrap around:-) In this case, use
>> if ( in->bv_len>  MYSIZE - len ) since len will be<= MYSIZE.
> No. You missed the point. The first part of the if will catch an
> outsized in->bv_len.

So does in->bv_len > MYSIZE - len, since you already have a bug
in the program if MYSIZE < len ("length of current buffer content").
If you worry about that use
	if ( len > MYSIZE || in->bv_len > MYSIZE - len )
which actually expresses what you are looking for: 'len' is valid,
range, and there is enough room left to append 'in'.

> There is never wraparound on any real world buffer sizes. E.g. in a 32
> bit platform you cannot have a 2GB data buffer because there's no
> address space left for the code or stack. Likewise for 64 bit.

Well, no space left for stack anyway.  I've used a machine where
code and data may have been separate address spaces.

However C allows size_t to be too small to reach the entire address
space.  That just means that C implementation won't support objects
larger than ((size_t)-1) bytes.  I wonder if Windows or DOS had a mode
like that (32-bit address space, 16 bit size_t), and for all I know
someone is doing it again today in a 32->64-bit transition mode.

> And of course anyone can see that
> 	in->bv_len + len > MYSIZE
> is exactly equivalent to
>	             len > MYSIZE - in->bv_len

After checking in->bv_len <= MYSIZE and unless in->bv_len + len wraps
around, yes.

Though if we run into size_t narrower than pointers, we might learn of
that in a ruder way: slap_sl_free() could fail to detect that the
pointer to be freed belongs to another context.
If the C implementation also ensures that no object can cross a
2**32-byte boundary, 'ptr1 < ptr2' need only compare the least
significant 32 bits of the pointers.  (DOS did that, at least.)