[vlc-devel] [PATCH] skins: fix signed overflow in bitmap allocation

[remi at remlab.net]

From: Rémi Denis-Courmont <remi at remlab.net>

Reported-by GitHub team members: @erik-krogh, @geoffw0, @MathiasVP, @owen-mc
---
 modules/gui/skins2/src/file_bitmap.cpp    |  7 ++++++-
 modules/gui/skins2/src/ft2_bitmap.cpp     | 10 ++++++++--
 modules/gui/skins2/src/generic_bitmap.cpp | 10 ++++++++--
 modules/gui/skins2/src/scaled_bitmap.cpp  |  8 ++++++--
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/modules/gui/skins2/src/file_bitmap.cpp b/modules/gui/skins2/src/file_bitmap.cpp
index 658bdc122f..c9505be370 100644
--- a/modules/gui/skins2/src/file_bitmap.cpp
+++ b/modules/gui/skins2/src/file_bitmap.cpp
@@ -38,6 +38,7 @@ FileBitmap::FileBitmap( intf_thread_t *pIntf, image_handler_t *pImageHandler,
 {
     video_format_t fmt_out;
     picture_t *pPic;
+    unsigned size;
 
     video_format_Init( &fmt_out, VLC_CODEC_RGBA );
 
@@ -61,7 +62,11 @@ FileBitmap::FileBitmap( intf_thread_t *pIntf, image_handler_t *pImageHandler,
     m_height = fmt_out.i_height;
     video_format_Clean( &fmt_out );
 
-    m_pData = new uint8_t[m_height * m_width * 4];
+    if (mul_overflow((unsigned)m_width, (unsigned)m_height, &size)
+     || mul_overflow(size, 4, &size))
+        throw std::bad_alloc();
+
+    m_pData = new uint8_t[size];
 
     // Compute the alpha layer
     uint8_t *pData = m_pData, *pSrc = pPic->p->p_pixels;
diff --git a/modules/gui/skins2/src/ft2_bitmap.cpp b/modules/gui/skins2/src/ft2_bitmap.cpp
index 08ce4a3421..7b1c468705 100644
--- a/modules/gui/skins2/src/ft2_bitmap.cpp
+++ b/modules/gui/skins2/src/ft2_bitmap.cpp
@@ -27,9 +27,15 @@
 FT2Bitmap::FT2Bitmap( intf_thread_t *pIntf, int width, int height ):
     GenericBitmap( pIntf ), m_width( width ), m_height( height )
 {
+    unsigned size;
+
+    if (mul_overflow((unsigned)width, (unsigned)height, &size)
+     || mul_overflow(size, 4, &size))
+        throw std::bad_alloc();
+
     // Allocate memory for the buffer
-    m_pData = new uint8_t[m_height * m_width * 4];
-    memset( m_pData, 0, m_height * m_width * 4 );
+    m_pData = new uint8_t[size];
+    memset(m_pData, 0, size);
 }
 
 
diff --git a/modules/gui/skins2/src/generic_bitmap.cpp b/modules/gui/skins2/src/generic_bitmap.cpp
index 60edecca40..a0c40f404a 100644
--- a/modules/gui/skins2/src/generic_bitmap.cpp
+++ b/modules/gui/skins2/src/generic_bitmap.cpp
@@ -57,8 +57,14 @@ BitmapImpl::BitmapImpl( intf_thread_t *pIntf, int width, int height,
     GenericBitmap( pIntf, nbFrames, fps, nbLoops ), m_width( width ),
     m_height( height ), m_pData( NULL )
 {
-    m_pData = new uint8_t[width * height * 4];
-    memset( m_pData, 0, width * height * 4 );
+    unsigned size;
+
+    if (mul_overflow((unsigned)width, (unsigned)height, &size)
+     || mul_overflow(size, 4, &size))
+        throw std::bad_alloc();
+
+    m_pData = new uint8_t[size];
+    memset(m_pData, 0, size);
 }
 
 
diff --git a/modules/gui/skins2/src/scaled_bitmap.cpp b/modules/gui/skins2/src/scaled_bitmap.cpp
index a85877b381..0bcbd02c4c 100644
--- a/modules/gui/skins2/src/scaled_bitmap.cpp
+++ b/modules/gui/skins2/src/scaled_bitmap.cpp
@@ -28,10 +28,14 @@ ScaledBitmap::ScaledBitmap( intf_thread_t *pIntf, const GenericBitmap &rBitmap,
                             int width, int height ):
     GenericBitmap( pIntf ), m_width( width ), m_height( height )
 {
-    // XXX We should check that width and height are positive...
+    unsigned size;
+
+    if (mul_overflow((unsigned)width, (unsigned)height, &size)
+     || mul_overflow(size, 4, &size))
+        throw std::bad_alloc();
 
     // Allocate memory for the buffer
-    m_pData = new uint8_t[m_height * m_width * 4];
+    m_pData = new uint8_t[size];
 
     int srcWidth = rBitmap.getWidth();
     int srcHeight = rBitmap.getHeight();
-- 
2.29.2