From 7733f676be1700a67eadad3edf24f3bd619e8d65 Mon Sep 17 00:00:00 2001
From: Reuben Cartwright <Reuben.Cartwright@arm.com>
Date: Thu, 12 Sep 2024 13:36:40 +0100
Subject: [PATCH] ota-agent-task: Fix bugs in ota_agent_task.c

This commit fixes usage of memcpy with potentially user-defined inputs,
without checking that the buffer could fit these inputs.

Signed-off-by: Reuben Cartwright <Reuben.Cartwright@arm.com>
---
 .../integration/src/ota_agent_task.c          | 42 +++++++++++++------
 release_changes/202409121339.change           |  2 +
 2 files changed, 32 insertions(+), 12 deletions(-)
 create mode 100644 release_changes/202409121339.change

diff --git a/components/aws_iot/ota_for_aws_iot_embedded_sdk/integration/src/ota_agent_task.c b/components/aws_iot/ota_for_aws_iot_embedded_sdk/integration/src/ota_agent_task.c
index 989f2f2..9d42e2d 100644
--- a/components/aws_iot/ota_for_aws_iot_embedded_sdk/integration/src/ota_agent_task.c
+++ b/components/aws_iot/ota_for_aws_iot_embedded_sdk/integration/src/ota_agent_task.c
@@ -470,6 +470,7 @@ STATIC OtaEventData_t * prvOTAEventBufferGet( void )
             {
                 eventBuffer[ ulIndex ].bufferUsed = true;
                 pFreeBuffer = &eventBuffer[ ulIndex ];
+                pFreeBuffer->dataLength = sizeof( pFreeBuffer->data );
                 break;
             }
         }
@@ -616,13 +617,20 @@ STATIC void prvMqttJobCallback( void * pvIncomingPublishCallbackContext,
 
     if( pData != NULL )
     {
-        memcpy( pData->data, pxPublishInfo->pPayload, pxPublishInfo->payloadLength );
-        pData->dataLength = pxPublishInfo->payloadLength;
-        eventMsg.eventId = OtaAgentEventReceivedJobDocument;
-        eventMsg.pEventData = pData;
+        if( ( size_t ) pData->dataLength >= pxPublishInfo->payloadLength )
+        {
+            memcpy( pData->data, pxPublishInfo->pPayload, pxPublishInfo->payloadLength );
+            pData->dataLength = pxPublishInfo->payloadLength;
+            eventMsg.eventId = OtaAgentEventReceivedJobDocument;
+            eventMsg.pEventData = pData;
 
-        /* Send job document received event. */
-        OTA_SignalEvent( &eventMsg );
+            /* Send job document received event. */
+            OTA_SignalEvent( &eventMsg );
+        }
+        else
+        {
+            LogError( ( "Error: OTA data buffers are too small for the Job message provided.\n" ) );
+        }
     }
     else
     {
@@ -666,13 +674,20 @@ STATIC void prvMqttDataCallback( void * pvIncomingPublishCallbackContext,
 
     if( pxData != NULL )
     {
-        memcpy( pxData->data, pxPublishInfo->pPayload, pxPublishInfo->payloadLength );
-        pxData->dataLength = pxPublishInfo->payloadLength;
-        eventMsg.eventId = OtaAgentEventReceivedFileBlock;
-        eventMsg.pEventData = pxData;
+        if( ( size_t ) pxData->dataLength >= pxPublishInfo->payloadLength )
+        {
+            memcpy( pxData->data, pxPublishInfo->pPayload, pxPublishInfo->payloadLength );
+            pxData->dataLength = pxPublishInfo->payloadLength;
+            eventMsg.eventId = OtaAgentEventReceivedFileBlock;
+            eventMsg.pEventData = pxData;
 
-        /* Send job document received event. */
-        OTA_SignalEvent( &eventMsg );
+            /* Send file block received event. */
+            OTA_SignalEvent( &eventMsg );
+        }
+        else
+        {
+            LogError( ( "Error: OTA data buffers are too small for the data message received.\n" ) );
+        }
     }
     else
     {
@@ -762,6 +777,9 @@ STATIC void prvMQTTUnsubscribeCompleteCallback( MQTTAgentCommandContext_t * pxCo
     }
 }
 
+/*
+ * Precondition: pTopicFilter is not null.
+ */
 STATIC OtaMqttStatus_t prvMQTTSubscribe( const char * pTopicFilter,
                                          uint16_t topicFilterLength,
                                          uint8_t ucQoS )
diff --git a/release_changes/202409121339.change b/release_changes/202409121339.change
new file mode 100644
index 0000000..53208e9
--- /dev/null
+++ b/release_changes/202409121339.change
@@ -0,0 +1,2 @@
+aws-iot-unit-test: Add tests for ota_agent_task.c.
+ota-agent-task: Debug unsafe memcpy usages in ota_agent_task.c.