Put some buffer overflow checks in.

Also, do a minor rework of StringHeap, and have it check its
assumptions more thoroughly.
This commit is contained in:
Stephen Williams 2019-11-21 18:35:43 -08:00
parent 7feb26ff6b
commit 7cf8bb9875
3 changed files with 48 additions and 9 deletions

View File

@ -29,10 +29,12 @@ static char **string_pool = NULL;
static unsigned string_pool_count = 0;
#endif
static const unsigned DEFAULT_CELL_SIZE = 0x10000;
StringHeap::StringHeap()
{
cell_base_ = 0;
cell_size_ = 0;
cell_ptr_ = 0;
}
@ -45,17 +47,37 @@ StringHeap::~StringHeap()
const char* StringHeap::add(const char*text)
{
unsigned len = strlen(text);
unsigned rem = cell_size_ - cell_ptr_;
unsigned rem = cell_base_==0? 0 : (DEFAULT_CELL_SIZE - cell_ptr_);
// Special case: huge items get their own allocation.
if ( (len+1) >= DEFAULT_CELL_SIZE ) {
char*buf = strdup(text);
#ifdef CHECK_WITH_VALGRIND
string_pool_count += 1;
string_pool = (char**)realloc(string_pool,
string_pool_count*sizeof(char**));
string_pool[string_pool_count-1] = buf;
#endif
return buf;
}
// If the current cell that I'm working through cannot hold
// the current string, then set it aside and get another cell
// to put the string into.
if (rem < (len+1)) {
// release any unused memory
// release any unused memory. This assumes that a
// realloc shrink of the memory region will return the
// same pointer.
if (rem > 0) {
char*old = cell_base_;
cell_base_ = (char*)realloc(cell_base_, cell_ptr_);
assert(cell_base_ != 0);
assert(cell_base_ == old);
}
// start new cell
cell_size_ = (len+1) > DEFAULT_CELL_SIZE ? len+1 : DEFAULT_CELL_SIZE;
cell_base_ = (char*)malloc(cell_size_);
cell_base_ = (char*)malloc(DEFAULT_CELL_SIZE);
cell_ptr_ = 0;
rem = DEFAULT_CELL_SIZE;
assert(cell_base_ != 0);
#ifdef CHECK_WITH_VALGRIND
string_pool_count += 1;
@ -65,12 +87,13 @@ const char* StringHeap::add(const char*text)
#endif
}
assert( (len+1) <= rem );
char*res = cell_base_ + cell_ptr_;
memcpy(res, text, len);
cell_ptr_ += len;
cell_base_[cell_ptr_++] = 0;
assert(cell_ptr_ <= cell_size_);
assert(cell_ptr_ <= DEFAULT_CELL_SIZE);
return res;
}

View File

@ -78,10 +78,7 @@ class StringHeap {
perm_string make(const char*);
private:
static const unsigned DEFAULT_CELL_SIZE = 0x10000;
char*cell_base_;
unsigned cell_size_;
unsigned cell_ptr_;
private: // not implemented

19
main.cc
View File

@ -606,6 +606,14 @@ static void read_iconfig_file(const char*ipath)
}
while (fgets(buf, sizeof buf, ifile) != 0) {
assert(strlen(buf) < sizeof buf);
if ((strlen(buf) == ((sizeof buf) - 1))
&& buf[sizeof buf -2] != '\n') {
cerr << "WARNING: Line buffer overflow reading iconfig file: "
<< ipath
<< "." << endl;
assert(0);
}
if (buf[0] == '#')
continue;
char*cp = strchr(buf, ':');
@ -808,6 +816,17 @@ static void read_sources_file(const char*path)
}
while (fgets(line_buf, sizeof line_buf, fd) != 0) {
// assertion test that we are not overflowing the line
// buffer. Really should make this more robust, but
// better to assert then go weird.
assert(strlen(line_buf) < sizeof line_buf);
if ((strlen(line_buf) == ((sizeof line_buf) - 1))
&& line_buf[sizeof line_buf -2] != '\n') {
cerr << "WARNING: Line buffer overflow reading sources file: "
<< path
<< "." << endl;
assert(0);
}
char*cp = line_buf + strspn(line_buf, " \t\r\b\f");
char*tail = cp + strlen(cp);
while (tail > cp) {