From c5cda1c311118acfb3ad818c9745d7c97c9d6b15 Mon Sep 17 00:00:00 2001
From: Michael Zillgith <michael.zillgith@mz-automation.de>
Date: Sat, 13 Aug 2022 20:36:18 +0200
Subject: [PATCH] - MMS server: fixed potential null pointer dereference when
 confirmedServiceResponse for fileOpen is received with invoke-id 0
 (LIB61850-348)

---
 src/mms/iso_mms/server/mms_file_service.c     | 16 ++++-----
 src/mms/iso_mms/server/mms_server.c           | 35 +++++++++++++++----
 .../iso_mms/server/mms_server_connection.c    | 14 +++++++-
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/src/mms/iso_mms/server/mms_file_service.c b/src/mms/iso_mms/server/mms_file_service.c
index fce656cc..7f56c171 100644
--- a/src/mms/iso_mms/server/mms_file_service.c
+++ b/src/mms/iso_mms/server/mms_file_service.c
@@ -622,23 +622,23 @@ mmsServerConnection_stopFileUploadTasks(MmsServerConnection self)
 
     for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) {
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+        Semaphore_wait(server->fileUploadTasks[i].taskLock);
+#endif
+
         if (server->fileUploadTasks[i].state != 0) {
 
             if (server->fileUploadTasks[i].connection == self) {
 
-#if (CONFIG_MMS_THREADLESS_STACK != 1)
-                Semaphore_wait(server->fileUploadTasks[i].taskLock);
-#endif
-
                 /* stop file upload task */
                 server->fileUploadTasks[i].state = MMS_FILE_UPLOAD_STATE_INTERRUPTED;
-
-#if (CONFIG_MMS_THREADLESS_STACK != 1)
-                Semaphore_post(server->fileUploadTasks[i].taskLock);
-#endif
             }
 
         }
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+        Semaphore_post(server->fileUploadTasks[i].taskLock);
+#endif
     }
 }
 
diff --git a/src/mms/iso_mms/server/mms_server.c b/src/mms/iso_mms/server/mms_server.c
index 390e0324..72c5492e 100644
--- a/src/mms/iso_mms/server/mms_server.c
+++ b/src/mms/iso_mms/server/mms_server.c
@@ -110,6 +110,20 @@ MmsServer_create(MmsDevice* device, TLSConfiguration tlsConfiguration)
         self->maxAssociationSpecificDataSets = CONFIG_MMS_MAX_NUMBER_OF_ASSOCIATION_SPECIFIC_DATA_SETS;
         self->maxDomainSpecificDataSets = CONFIG_MMS_MAX_NUMBER_OF_DOMAIN_SPECIFIC_DATA_SETS;
 #endif /* (CONFIG_MMS_SERVER_CONFIG_SERVICES_AT_RUNTIME == 1) */
+
+#if (MMS_OBTAIN_FILE_SERVICE == 1)
+        {
+            int i;
+
+            for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) {
+                self->fileUploadTasks[i].state = 0;
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+                self->fileUploadTasks[i].taskLock = Semaphore_create(1);
+#endif /* (CONFIG_MMS_THREADLESS_STACK != 1) */
+            }   
+        }
+#endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */
     }
 
     return self;
@@ -295,17 +309,24 @@ MmsServer_getObtainFileTask(MmsServer self)
 
     for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) {
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+        Semaphore_wait(self->fileUploadTasks[i].taskLock);
+#endif
+
         if (self->fileUploadTasks[i].state == 0) {
             self->fileUploadTasks[i].state = 1;
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-            if (self->fileUploadTasks[i].taskLock == NULL)
-                self->fileUploadTasks[i].taskLock = Semaphore_create(1);
+            Semaphore_post(self->fileUploadTasks[i].taskLock);
 #endif
 
             return &(self->fileUploadTasks[i]);
         }
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+        Semaphore_post(self->fileUploadTasks[i].taskLock);
+#endif
+
     }
 
     return NULL;
@@ -728,19 +749,19 @@ MmsServer_handleBackgroundTasks(MmsServer self)
     int i;
     for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++)
     {
-        if (self->fileUploadTasks[i].state != 0) {
-
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-            Semaphore_wait(self->fileUploadTasks[i].taskLock);
+        Semaphore_wait(self->fileUploadTasks[i].taskLock);
 #endif
 
+        if (self->fileUploadTasks[i].state != 0) {
+
             if (self->fileUploadTasks[i].state != 0)
                 mmsServer_fileUploadTask(self, &(self->fileUploadTasks[i]));
+        }
 
 #if (CONFIG_MMS_THREADLESS_STACK != 1)
-            Semaphore_post(self->fileUploadTasks[i].taskLock);
+        Semaphore_post(self->fileUploadTasks[i].taskLock);
 #endif
-        }
     }
 
 #endif /* (MMS_OBTAIN_FILE_SERVICE == 1) */
diff --git a/src/mms/iso_mms/server/mms_server_connection.c b/src/mms/iso_mms/server/mms_server_connection.c
index 6cce1696..b14ecc3a 100644
--- a/src/mms/iso_mms/server/mms_server_connection.c
+++ b/src/mms/iso_mms/server/mms_server_connection.c
@@ -434,15 +434,27 @@ handleConfirmedErrorPdu(
             int i;
             for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) {
 
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+                Semaphore_wait(self->server->fileUploadTasks[i].taskLock);
+#endif
+
                 if (self->server->fileUploadTasks[i].state != MMS_FILE_UPLOAD_STATE_NOT_USED) {
 
                     if (self->server->fileUploadTasks[i].lastRequestInvokeId == invokeId) {
 
                         self->server->fileUploadTasks[i].state = MMS_FILE_UPLOAD_STATE_SEND_OBTAIN_FILE_ERROR_SOURCE;
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+                        Semaphore_post(self->server->fileUploadTasks[i].taskLock);
+#endif
                         return;
                     }
 
                 }
+
+#if (CONFIG_MMS_THREADLESS_STACK != 1)
+                Semaphore_post(self->server->fileUploadTasks[i].taskLock);
+#endif
             }
         }
 
@@ -458,7 +470,7 @@ getUploadTaskByInvokeId(MmsServer mmsServer, uint32_t invokeId)
 {
     int i;
     for (i = 0; i < CONFIG_MMS_SERVER_MAX_GET_FILE_TASKS; i++) {
-        if (mmsServer->fileUploadTasks[i].lastRequestInvokeId == invokeId)
+        if ((mmsServer->fileUploadTasks[i].state != 0) && (mmsServer->fileUploadTasks[i].lastRequestInvokeId == invokeId))
             return &(mmsServer->fileUploadTasks[i]);
     }