From a095c16139698aed32d9e18fc07d4f9c1e7491ab Mon Sep 17 00:00:00 2001 From: Mikael Bourhis Date: Thu, 22 Oct 2020 11:36:33 +0200 Subject: [PATCH] IED Server/Goose: Fix the 'Goose Ethernet Interface Id' allocation Issue: In the case of 'GooseInterfaceId' initialization for all GOOSE publishers (API function: IedServer_setGooseInterfaceId()), only the pointer of the character string is saved and used for the next operations. If the corresponding memory is cleared or overwritten in the application program, a segfault may occurred in libiec61850. Proof: In the 'server_example_goose.c' example, if we used a temporary allocated memory to store the 'ethernetIfcId', the bug will occur: /* set GOOSE interface for all GOOSE publishers (GCBs) */ + unsigned int ethernet_interface_id_length = strnlen(ethernetIfcID, 32); + char * tmp_ethernet_interface_id = calloc(sizeof(char), ethernet_interface_id_length + 1); + memcpy(tmp_ethernet_interface_id, ethernetIfcID, ethernet_interface_id_length); + IedServer_setGooseInterfaceId(iedServer, tmp_ethernet_interface_id); + + explicit_bzero(tmp_ethernet_interface_id, ethernet_interface_id_length); + free(tmp_ethernet_interface_id); (This issue was discovered during the rewriting of 'server_example_goose' in Python, with the Swig wrapper: the allocated memory for 'ethernetIfcID' parameter in the Swig wrapper is automatically reused) Fix: The 'gooseInterfaceId' attribute of 'MmsMapping' in libiec61850 must be a new allocated memory (with the use of 'StringUtils_copyString()') and deallocated in the 'MmsMapping_destroy()' function. --- src/iec61850/server/impl/ied_server.c | 2 +- src/iec61850/server/mms_mapping/mms_mapping.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/iec61850/server/impl/ied_server.c b/src/iec61850/server/impl/ied_server.c index 16ec66f4..05a7eedd 100644 --- a/src/iec61850/server/impl/ied_server.c +++ b/src/iec61850/server/impl/ied_server.c @@ -1692,6 +1692,6 @@ void IedServer_setGooseInterfaceId(IedServer self, const char* interfaceId) { #if (CONFIG_INCLUDE_GOOSE_SUPPORT == 1) - self->mmsMapping->gooseInterfaceId = interfaceId; + self->mmsMapping->gooseInterfaceId = StringUtils_copyString(interfaceId); #endif } diff --git a/src/iec61850/server/mms_mapping/mms_mapping.c b/src/iec61850/server/mms_mapping/mms_mapping.c index 985bcc41..c326b76c 100644 --- a/src/iec61850/server/mms_mapping/mms_mapping.c +++ b/src/iec61850/server/mms_mapping/mms_mapping.c @@ -2015,6 +2015,7 @@ MmsMapping_destroy(MmsMapping* self) #if (CONFIG_INCLUDE_GOOSE_SUPPORT == 1) LinkedList_destroyDeep(self->gseControls, (LinkedListValueDeleteFunction) MmsGooseControlBlock_destroy); + if (self->gooseInterfaceId) GLOBAL_FREEMEM(self->gooseInterfaceId); #endif #if (CONFIG_IEC61850_SAMPLED_VALUES_SUPPORT == 1)