CVE-2019-25051: Out-of-bounds Write (CWE-787)
value: objstack in GNU Aspell 0.60.8 has a heap-based buffer overflow in acommon::ObjStack::dup_top (called from acommon::StringMap::add and acommon::Config::lookup_list).
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.8
Published: 2021-07-20T07:15Z
Repository: https://github.com/gnuaspell/aspell
Language: C++
Files changed: common/objstack.hpp
Commit date: 2019-12-21 21:36:38 +0000
Commit message: objstack: assert that the alloc size will fit within a chunk
to prevent a buffer overflow

Bug found using OSS-Fuze.

Diff: diff --git a/common/objstack.hpp b/common/objstack.hpp
index 3997bf7..bd97ccd 100644
--- a/common/objstack.hpp
+++ b/common/objstack.hpp
@@ -3,10 +3,11 @@
 #define ACOMMON_OBJSTACK__HPP
 
 #include "parm_string.hpp"
 #include <stdlib.h>
 #include <assert.h>
+#include <stddef.h>
 
 namespace acommon {
 
 class ObjStack
 {
@@ -24,10 +25,16 @@ class ObjStack
   byte * top;
   byte * bottom;
   byte * temp_end;
   void setup_chunk();
   void new_chunk();
+  bool will_overflow(size_t sz) const {
+    return offsetof(Node,data) + sz > chunk_size;
+  }
+  void check_size(size_t sz) {
+    assert(!will_overflow(sz));
+  }
 
   ObjStack(const ObjStack &);
   void operator=(const ObjStack &);
 
   void align_bottom(size_t align) {
@@ -54,21 +61,21 @@ public:
   // insert objects with a multiple of min_align than it will always
   // me aligned as such.
   void * alloc_bottom(size_t size)  {
     byte * tmp = bottom;
     bottom += size;
-    if (bottom > top) {new_chunk(); tmp = bottom; bottom += size;}
+    if (bottom > top) {check_size(size); new_chunk(); tmp = bottom; bottom += size;}
     return tmp;
   }
   // This alloc_bottom will insure that the object is aligned based on the
   // alignment given.
   void * alloc_bottom(size_t size, size_t align) 
   {loop:
     align_bottom(align);
     byte * tmp = bottom;
     bottom += size;
-    if (bottom > top) {new_chunk(); goto loop;}
+    if (bottom > top) {check_size(size); new_chunk(); goto loop;}
     return tmp;
   }
   char * dup_bottom(ParmString str) {
     return (char *)memcpy(alloc_bottom(str.size() + 1), 
                           str.str(), str.size() + 1);
@@ -77,20 +84,20 @@ public:
   // This alloc_bottom does NOT check alignment.  However, if you
   // always insert objects with a multiple of min_align than it will
   // always be aligned as such.
   void * alloc_top(size_t size) {
     top -= size;
-    if (top < bottom) {new_chunk(); top -= size;}
+    if (top < bottom) {check_size(size); new_chunk(); top -= size;}
     return top;
   }
   // This alloc_top will insure that the object is aligned based on
   // the alignment given.
   void * alloc_top(size_t size, size_t align) 
   {loop:
     top -= size;
     align_top(align);
-    if (top < bottom) {new_chunk(); goto loop;}
+    if (top < bottom) {check_size(size); new_chunk(); goto loop;}
     return top;
   }
   char * dup_top(ParmString str) {
     return (char *)memcpy(alloc_top(str.size() + 1), 
                           str.str(), str.size() + 1);
@@ -115,10 +122,11 @@ public:
     return temp_end - bottom;
   }
   void * alloc_temp(size_t size) {
     temp_end = bottom + size;
     if (temp_end > top) {
+      check_size(size);
       new_chunk();
       temp_end = bottom + size;
     }
     return bottom;
   }
@@ -129,10 +137,11 @@ public:
     if (bottom + size <= top) {
       temp_end = bottom + size;
     } else {
       size_t s = temp_end - bottom;
       byte * p = bottom;
+      check_size(size);
       new_chunk();
       memcpy(bottom, p, s);
       temp_end = bottom + size;
     }
     return bottom;
@@ -148,10 +157,11 @@ public:
     if (bottom + size <= top) {
       temp_end = bottom + size;
     } else {
       size_t s = temp_end - bottom;
       byte * p = bottom;
+      check_size(size);
       new_chunk();
       memcpy(bottom, p, s);
       temp_end = bottom + size;
     }
     return bottom + old_size;

Vulnerable code:     if (bottom > top) {new_chunk(); tmp = bottom; bottom += size;}
    if (bottom > top) {new_chunk(); goto loop;}
    if (top < bottom) {new_chunk(); top -= size;}
    if (top < bottom) {new_chunk(); goto loop;}
Fixed code: #include <stddef.h>
  bool will_overflow(size_t sz) const {
    return offsetof(Node,data) + sz > chunk_size;
  }
  void check_size(size_t sz) {
    assert(!will_overflow(sz));
  }
    if (bottom > top) {check_size(size); new_chunk(); tmp = bottom; bottom += size;}
    if (bottom > top) {check_size(size); new_chunk(); goto loop;}
    if (top < bottom) {check_size(size); new_chunk(); top -= size;}
    if (top < bottom) {check_size(size); new_chunk(); goto loop;}
      check_size(size);
      check_size(size);
      check_size(size);