Fix MAVLink message bounds validation vulnerabilities (Stable_V5.0 backport)

.gitignore

@@ -1,6 +1,7 @@
 .idea/
 .vscode/
 .cache/
+.ccache/
 cmake-build-*/
 out/
 libs/lib/Frameworks/GStreamer.framework

src/MAVLink/ImageProtocolManager.cc

@@ -10,6 +10,8 @@
 #include "ImageProtocolManager.h"
 #include "QGCLoggingCategory.h"
 
+#include <cstring>
+
 QGC_LOGGING_CATEGORY(ImageProtocolManagerLog, "qgc.mavlink.imageprotocolmanager")
 
 ImageProtocolManager::ImageProtocolManager(QObject *parent)
@@ -58,6 +60,27 @@ void ImageProtocolManager::mavlinkMessageReceived(const mavlink_message_t &messa
         _imageBytes.clear();
         mavlink_msg_data_transmission_handshake_decode(&message, &_imageHandshake);
         qCDebug(ImageProtocolManagerLog) << QStringLiteral("DATA_TRANSMISSION_HANDSHAKE: type(%1) width(%2) height (%3)").arg(_imageHandshake.type).arg(_imageHandshake.width).arg(_imageHandshake.height);
+
+        // Validate handshake fields to prevent out-of-bounds writes on subsequent ENCAPSULATED_DATA
+        static constexpr uint32_t kMaxImageSize = 1u * 1024u * 1024u; // 1 MB upper bound (optical flow images are typically small grayscale frames)
+        if (_imageHandshake.size == 0 || _imageHandshake.payload == 0 || _imageHandshake.packets == 0) {
+            qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: Invalid field(s) - size:" << _imageHandshake.size
+                                               << "payload:" << _imageHandshake.payload << "packets:" << _imageHandshake.packets;
+            _imageHandshake = {};
+            break;
+        }
+        if (_imageHandshake.size > kMaxImageSize) {
+            qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: Image size exceeds limit. size:" << _imageHandshake.size;
+            _imageHandshake = {};
+            break;
+        }
+        if (_imageHandshake.payload > sizeof(mavlink_encapsulated_data_t::data)) {
+            qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: payload exceeds ENCAPSULATED_DATA data field size. payload:" << _imageHandshake.payload;
+            _imageHandshake = {};
+            break;
+        }
+
+        _imageBytes.resize(_imageHandshake.size, '\0');
         break;
     }
     case MAVLINK_MSG_ID_ENCAPSULATED_DATA:
@@ -70,16 +93,17 @@ void ImageProtocolManager::mavlinkMessageReceived(const mavlink_message_t &messa
         mavlink_encapsulated_data_t encapsulatedData;
         mavlink_msg_encapsulated_data_decode(&message, &encapsulatedData);
 
-        uint32_t bytePosition = encapsulatedData.seqnr * _imageHandshake.payload;
+        const uint32_t bytePosition = static_cast<uint32_t>(encapsulatedData.seqnr) * _imageHandshake.payload;
         if (bytePosition >= _imageHandshake.size) {
             qCWarning(ImageProtocolManagerLog) << "ENCAPSULATED_DATA: seqnr is past end of image size. seqnr:" << encapsulatedData.seqnr << "_imageHandshake.size:" << _imageHandshake.size;
             break;
         }
 
-        for (uint8_t i = 0; i < _imageHandshake.payload; i++) {
-            _imageBytes[bytePosition] = encapsulatedData.data[i];
-            bytePosition++;
-        }
+        // Clamp the number of bytes to copy so we never write past the declared image size
+        const uint32_t bytesRemaining = _imageHandshake.size - bytePosition;
+        const uint32_t bytesToCopy = qMin(static_cast<uint32_t>(_imageHandshake.payload), bytesRemaining);
+
+        (void) memcpy(_imageBytes.data() + bytePosition, encapsulatedData.data, bytesToCopy);
 
         // We use the packets field to track completion
         _imageHandshake.packets--;

src/Vehicle/FTPManager.cc

@@ -237,6 +237,12 @@ void FTPManager::_mavlinkMessageReceived(const mavlink_message_t& message)
 
     MavlinkFTP::Request* request = (MavlinkFTP::Request*)&data.payload[0];
 
+    // Clamp hdr.size to the actual data array bounds to prevent over-reads
+    if (request->hdr.size > sizeof(request->data)) {
+        qCWarning(FTPManagerLog) << "_mavlinkMessageReceived: hdr.size exceeds data array, discarding." << request->hdr.size;
+        return;
+    }
+
     // Ignore old/reordered packets (handle wrap-around properly)
     uint16_t actualIncomingSeqNumber = request->hdr.seqNumber;
     if ((uint16_t)((_expectedIncomingSeqNumber - 1) - actualIncomingSeqNumber) < (std::numeric_limits<uint16_t>::max()/2)) {

src/Vehicle/Vehicle.cc

@@ -605,7 +605,7 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes
         mavlink_serial_control_t ser;
         mavlink_msg_serial_control_decode(&message, &ser);
         if (static_cast<size_t>(ser.count) > sizeof(ser.data)) {
-            qWarning() << "Invalid count for SERIAL_CONTROL, discarding." << ser.count;
+            qCWarning(VehicleLog) << "Invalid count for SERIAL_CONTROL, discarding." << ser.count;
         } else {
             emit mavlinkSerialControl(ser.device, ser.flags, ser.timeout, ser.baudrate,
                     QByteArray(reinterpret_cast<const char*>(ser.data), ser.count));
@@ -643,7 +643,11 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes
     {
         mavlink_log_data_t log{};
         mavlink_msg_log_data_decode(&message, &log);
-        emit logData(log.ofs, log.id, log.count, log.data);
+        if (static_cast<size_t>(log.count) > sizeof(log.data)) {
+            qCWarning(VehicleLog) << "Invalid count for LOG_DATA, discarding." << log.count;
+        } else {
+            emit logData(log.ofs, log.id, log.count, log.data);
+        }
         break;
     }
     case MAVLINK_MSG_ID_MESSAGE_INTERVAL:

src/VideoManager/VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt

@@ -4,7 +4,7 @@ if(MACOS)
     # Using FindGStreamer.cmake is currently bypassed on MACOS since it doesn't work
     # So for now we hack in a simple hardwired setup which does work
     find_library(GSTREAMER_FRAMEWORK GStreamer)
-    set(GST_PLUGINS_VERSION 1.24.12)
+    set(GST_PLUGINS_VERSION 1.24.13)
     set(GSTREAMER_FRAMEWORK_PATH "/Library/Frameworks/GStreamer.framework")
     if(NOT EXISTS "${GSTREAMER_FRAMEWORK_PATH}")
         message(FATAL_ERROR "GStreamer.framework not found at ${GSTREAMER_FRAMEWORK_PATH}. Install GStreamer using tools/setup/install-dependencies-osx.sh script")
@@ -23,7 +23,7 @@ endif()
 ################################################################################
 
 if(GStreamer_VERSION VERSION_GREATER_EQUAL 1.22)
-    # Use Latest Revisions for each minor version: 1.20.7, 1.22.12, 1.24.12, 1.26.2
+    # Use Latest Revisions for each minor version: 1.20.7, 1.22.12, 1.24.13, 1.26.2
     string(REPLACE "." ";" GST_VERSION_LIST ${GStreamer_VERSION})
     list(GET GST_VERSION_LIST 0 GST_VERSION_MAJOR)
     list(GET GST_VERSION_LIST 1 GST_VERSION_MINOR)

tools/setup/install-dependencies-osx.sh

@@ -12,7 +12,7 @@ brew install cmake ninja ccache git pkgconf create-dmg
 
 # Install GStreamer
 GST_URL=https://gstreamer.freedesktop.org/data/pkg/osx
-GST_VERSION=1.24.12
+GST_VERSION=1.24.13
 GST_PKG=gstreamer-1.0-$GST_VERSION-universal.pkg
 GST_DEV_PKG=gstreamer-1.0-devel-$GST_VERSION-universal.pkg
 pushd $TMPDIR