[Top][All Lists]

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

[aspell-devel] Memory overrun bug in aspell 0.60's String class

From: Brett Wilson
Subject: [aspell-devel] Memory overrun bug in aspell 0.60's String class
Date: Mon, 6 Sep 2004 22:18:46 -0700


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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]