tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Changing fgetstr/fgetln to use getdelim



Matthew Mondor wrote:
On Sun, 20 Sep 2009 16:51:05 +0100
Roy Marples <roy%marples.name@localhost> wrote:

One thing I did notice though is that __slbexpand expands the buffer on upto a size_t, but the place holder on the struct is only an int and doesn't have any bounds checking. Surely this is a potential overflow?

I didn't check the whole file (only the diff), but it appears that the
old expand code added more bytes than requested, possibly in attempt
not to realloc(3) too often.  A common practice is to simply double the
buffer for better performance, although this might be overkill.

Yes, you are correct. But consider this

/* stdio buffers */
struct __sbuf {
        unsigned char *_base;
        int     _size;
};

It's trying to store a size_t in _size without any range checking, which in my eyes is asking for trouble. Here's the part of my patch which fixes this.

+       if (size > INT_MAX) {
+               fp->_lb._size = INT_MAX;
+               errno = EOVERFLOW;
+               goto error;
        }

Thanks

Roy


Home | Main Index | Thread Index | Old Index