CAN: Fix overreading QByteArray buffer

The old code did not take the size of the payload
QByteArray into account, so that for small payloads
read accesses outside the QByteArray data field
occurred.

While this should be no big problem in reality,
memory checkers like Address Sanitizer will report
such issues.

We now only copy the bytes the payload QByteArray
really provides. We can do this, as every derived
QCanBusDevice::writeFrame() method already does a
QCanBusFrame::isValid() check before enqueuing the
outgoing frames, so that the maximum length of the
payload field is always guaranteed, for CAN 2.0
as well as CAN FD.

[ChangeLog][CAN] Fixed potential read buffer
overflows when sending CAN frames in diverse CAN
backends which were detected by Address Sanitizer.

Fixes: QTBUG-94695
Change-Id: I2e45f6c14ae0fe88ba83f52dd5db4ffe24dada58
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
(cherry picked from commit 8eaa91e1edd7a2b5849a18487a08c440b2b7a27a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 18 insertions, 15 deletions

diff --git a/src/plugins/canbus/peakcan/peakcanbackend.cpp b/src/plugins/canbus/peakcan/peakcanbackend.cpp
index 3b5ca6c..45d8023 100644
--- a/src/plugins/canbus/peakcan/peakcanbackend.cpp
+++ b/src/plugins/canbus/peakcan/peakcanbackend.cpp
@@ -635,13 +635,13 @@ void PeakCanBackendPrivate::startWrite()
 
     const QCanBusFrame frame = q->dequeueOutgoingFrame();
     const QByteArray payload = frame.payload();
+    const qsizetype payloadSize = payload.size();
     TPCANStatus st = PCAN_ERROR_OK;
 
     if (isFlexibleDatarateEnabled) {
-        const int size = payload.size();
         TPCANMsgFD message = {};
         message.ID = frame.frameId();
-        message.DLC = sizeToDlc(size);
+        message.DLC = sizeToDlc(payloadSize);
         message.MSGTYPE = frame.hasExtendedFrameFormat() ? PCAN_MESSAGE_EXTENDED
                                                          : PCAN_MESSAGE_STANDARD;
 
@@ -653,7 +653,7 @@ void PeakCanBackendPrivate::startWrite()
         if (frame.frameType() == QCanBusFrame::RemoteRequestFrame)
             message.MSGTYPE |= PCAN_MESSAGE_RTR; // we do not care about the payload
         else
-            ::memcpy(message.DATA, payload.constData(), sizeof(message.DATA));
+            ::memcpy(message.DATA, payload.constData(), payloadSize);
         st = ::CAN_WriteFD(channelIndex, &message);
     } else if (frame.hasFlexibleDataRateFormat()) {
         const char errorString[] = "Cannot send CAN FD frame format as CAN FD is not enabled.";
@@ -662,14 +662,14 @@ void PeakCanBackendPrivate::startWrite()
     } else {
         TPCANMsg message = {};
         message.ID = frame.frameId();
-        message.LEN = static_cast<quint8>(payload.size());
+        message.LEN = static_cast<quint8>(payloadSize);
         message.MSGTYPE = frame.hasExtendedFrameFormat() ? PCAN_MESSAGE_EXTENDED
                                                          : PCAN_MESSAGE_STANDARD;
 
         if (frame.frameType() == QCanBusFrame::RemoteRequestFrame)
             message.MSGTYPE |= PCAN_MESSAGE_RTR; // we do not care about the payload
         else
-            ::memcpy(message.DATA, payload.constData(), sizeof(message.DATA));
+            ::memcpy(message.DATA, payload.constData(), payloadSize);
         st = ::CAN_Write(channelIndex, &message);
     }
 
diff --git a/src/plugins/canbus/systeccan/systeccanbackend.cpp b/src/plugins/canbus/systeccan/systeccanbackend.cpp
index c1520a5..4c251b5 100644
--- a/src/plugins/canbus/systeccan/systeccanbackend.cpp
+++ b/src/plugins/canbus/systeccan/systeccanbackend.cpp
@@ -372,18 +372,19 @@ void SystecCanBackendPrivate::startWrite()
 
     const QCanBusFrame frame = q->dequeueOutgoingFrame();
     const QByteArray payload = frame.payload();
+    const qsizetype payloadSize = payload.size();
 
     tCanMsgStruct message = {};
 
     message.m_dwID = frame.frameId();
-    message.m_bDLC = quint8(payload.size());
+    message.m_bDLC = quint8(payloadSize);
 
     message.m_bFF = frame.hasExtendedFrameFormat() ? USBCAN_MSG_FF_EXT : USBCAN_MSG_FF_STD;
 
     if (frame.frameType() == QCanBusFrame::RemoteRequestFrame)
         message.m_bFF |= USBCAN_MSG_FF_RTR; // remote request frame without payload
     else
-        ::memcpy(message.m_bData, payload.constData(), sizeof(message.m_bData));
+        ::memcpy(message.m_bData, payload.constData(), payloadSize);
 
     const UCANRET result = ::UcanWriteCanMsgEx(handle, channel, &message, nullptr);
     if (Q_UNLIKELY(result != USBCAN_SUCCESSFUL))
diff --git a/src/plugins/canbus/tinycan/tinycanbackend.cpp b/src/plugins/canbus/tinycan/tinycanbackend.cpp
index 3600f8e..c990f91 100644
--- a/src/plugins/canbus/tinycan/tinycanbackend.cpp
+++ b/src/plugins/canbus/tinycan/tinycanbackend.cpp
@@ -352,21 +352,22 @@ void TinyCanBackendPrivate::startWrite()
 
     const QCanBusFrame frame = q->dequeueOutgoingFrame();
     const QByteArray payload = frame.payload();
+    const qsizetype payloadSize = payload.size();
 
     TCanMsg message = {};
 
-    if (Q_UNLIKELY(payload.size() > int(sizeof(message.Data.Bytes)))) {
-        qCWarning(QT_CANBUS_PLUGINS_TINYCAN, "Cannot write frame with payload size %d.", int(payload.size()));
+    if (Q_UNLIKELY(payloadSize > qsizetype(sizeof(message.Data.Bytes)))) {
+        qCWarning(QT_CANBUS_PLUGINS_TINYCAN, "Cannot write frame with payload size %d.", int(payloadSize));
     } else {
         message.Id = frame.frameId();
-        message.Flags.Flag.Len = payload.size();
+        message.Flags.Flag.Len = payloadSize;
         message.Flags.Flag.Error = (frame.frameType() == QCanBusFrame::ErrorFrame);
         message.Flags.Flag.RTR = (frame.frameType() == QCanBusFrame::RemoteRequestFrame);
         message.Flags.Flag.TxD = 1;
         message.Flags.Flag.EFF = frame.hasExtendedFrameFormat();
 
         const qint32 messagesToWrite = 1;
-        ::memcpy(message.Data.Bytes, payload.constData(), sizeof(message.Data.Bytes));
+        ::memcpy(message.Data.Bytes, payload.constData(), payloadSize);
         const int ret = ::CanTransmit(channelIndex, &message, messagesToWrite);
         if (Q_UNLIKELY(ret < 0))
             q->setError(systemErrorString(ret), QCanBusDevice::CanBusError::WriteError);
diff --git a/src/plugins/canbus/vectorcan/vectorcanbackend.cpp b/src/plugins/canbus/vectorcan/vectorcanbackend.cpp
index 85cb17f..f2ee0af 100644
--- a/src/plugins/canbus/vectorcan/vectorcanbackend.cpp
+++ b/src/plugins/canbus/vectorcan/vectorcanbackend.cpp
@@ -343,6 +343,7 @@ void VectorCanBackendPrivate::startWrite()
 
     const QCanBusFrame frame = q->dequeueOutgoingFrame();
     const QByteArray payload = frame.payload();
+    const qsizetype payloadSize = payload.size();
 
     quint32 eventCount = 1;
     XLstatus status = XL_ERROR;
@@ -356,13 +357,13 @@ void VectorCanBackendPrivate::startWrite()
         if (frame.hasExtendedFrameFormat())
             msg.id |= XL_CAN_EXT_MSG_ID;
 
-        msg.dlc = payload.size();
+        msg.dlc = payloadSize;
         if (frame.hasFlexibleDataRateFormat())
             msg.flags = XL_CAN_TXMSG_FLAG_EDL;
         if (frame.frameType() == QCanBusFrame::RemoteRequestFrame)
             msg.flags |= XL_CAN_TXMSG_FLAG_RTR; // we do not care about the payload
         else
-            ::memcpy(msg.data, payload.constData(), sizeof(msg.data));
+            ::memcpy(msg.data, payload.constData(), payloadSize);
 
         status = ::xlCanTransmitEx(portHandle, channelMask, eventCount, &eventCount, &event);
     } else {
@@ -374,14 +375,14 @@ void VectorCanBackendPrivate::startWrite()
         if (frame.hasExtendedFrameFormat())
             msg.id |= XL_CAN_EXT_MSG_ID;
 
-        msg.dlc = payload.size();
+        msg.dlc = payloadSize;
 
         if (frame.frameType() == QCanBusFrame::RemoteRequestFrame)
             msg.flags |= XL_CAN_MSG_FLAG_REMOTE_FRAME; // we do not care about the payload
         else if (frame.frameType() == QCanBusFrame::ErrorFrame)
             msg.flags |= XL_CAN_MSG_FLAG_ERROR_FRAME; // we do not care about the payload
         else
-            ::memcpy(msg.data, payload.constData(), sizeof(msg.data));
+            ::memcpy(msg.data, payload.constData(), payloadSize);
 
         status = ::xlCanTransmit(portHandle, channelMask, &eventCount, &event);
     }