Backport memory fixes found by ASAN (#340)

* Fix heap overflow when reading (#337) PF_RGB is 3 bytes. But later on Ogre2SelectionBuffer::OnSelectionClick will try to read 4 bytes from it. Fixed by ensuring it's always at least 4 bytes and zero-initializing those 4 bytes. Signed-off-by: Matias N. Goldberg <[email protected]> Co-authored-by: Ian Chen <[email protected]> * Fix new [] / delete mismatch (#338) Using a custom deallocator to avoid breaking ABI Alternatively C++17 supports the following syntax, which was not used: `typedef std::shared_ptr<unsigned char[]> DataPtr;` Signed-off-by: Matias N. Goldberg <[email protected]> Co-authored-by: darksylinc <[email protected]>
gazebosim · Jun 15, 2021 · a56c834 · a56c834
1 parent fe96179
commit a56c834
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/ogre2/src/Ogre2SelectionBuffer.cc b/ogre2/src/Ogre2SelectionBuffer.cc
@@ -175,9 +175,12 @@ void Ogre2SelectionBuffer::CreateRTTBuffer()
   const_cast<Ogre::CompositorPassSceneDef *>(scenePass)->mVisibilityMask =
       IGN_VISIBILITY_SELECTABLE;
 
-  // buffer to store render texture data
-  size_t bufferSize = Ogre::PixelUtil::getMemorySize(width, height, 1, format);
+  // buffer to store render texture data. Ensure it's at least 4 bytes
+  size_t bufferSize = std::max<size_t>(
+      Ogre::PixelUtil::getMemorySize(width, height, 1, format),
+      4u);
   this->dataPtr->buffer = new uint8_t[bufferSize];
+  memset(this->dataPtr->buffer, 0, 4u);
   this->dataPtr->pixelBox = new Ogre::PixelBox(width,
       height, 1, format, this->dataPtr->buffer);
 }

diff --git a/src/Image.cc b/src/Image.cc
@@ -19,6 +19,16 @@
 using namespace ignition;
 using namespace rendering;
 
+//////////////////////////////////////////////////
+template <class T>
+struct ArrayDeleter
+{
+  void operator () (T const * p)
+  {
+    delete [] p;
+  }
+};
+
 //////////////////////////////////////////////////
 Image::Image(unsigned int _width, unsigned int _height,
   PixelFormat _format) :
@@ -27,7 +37,7 @@ Image::Image(unsigned int _width, unsigned int _height,
 {
   this->format = PixelUtil::Sanitize(_format);
   unsigned int size = this->MemorySize();
-  this->data = DataPtr(new unsigned char[size]);
+  this->data = DataPtr(new unsigned char[size], ArrayDeleter<unsigned char>());
 }
 
 //////////////////////////////////////////////////