Memory overrun bug in aspell 0.60's String class

Brett Wilson <[email protected]>
Newsgroups gmane.comp.gnu.aspell.devel
Message-ID <[email protected]>
Hi,

I've discovered a bug in the memory allocation of the String class.
This seems pretty significant because this class is used everywhere in
the library. Here's the function in question (common/string.cpp:23):

  void String::reserve_i(size_t s)
  {
    size_t old_size = end_ - begin_;
    size_t new_size = (storage_end_ - begin_) * 3 / 2;
    if (new_size < 64) new_size = 64;
    if (new_size + 1 < s) new_size = s + 1;  <=========
    if (old_size == 0) {
      if (begin_) free(begin_);
      begin_ = (char *)malloc(new_size);
    } else {
      begin_ = (char *)realloc(begin_, new_size);
    }
    end_ = begin_ + old_size;
    storage_end_ = begin_ + new_size;
  }

The problem is if the initial buffer is small (<64 so it is expanded
to 64 in the 3rd line) and s (the length we need to make room for) is
64 or 65. In this case, new_size will still be 64, not leaving room
for the last character of s and possibly the null terminator. Yikes!

What was really meant was:

    if (new_size - 1 < s) new_size = s + 1;


Also, in String operator+ (string.hpp:397) the names of the two
arguments are switched. The function works correctly, as the names are
used consistently wrong, but I was pretty confused when I stepped
through this function while tracking down the above error. I take
'lhs' and 'rhs' to mean "Left/Right Hand Side", but 'rhs' actually
ends up on the left side of the concatenated string.

Brett Wilson
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.