From 1f52be9ddeae00e69cd43e4cac3cb4f0c880c4f0 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Mon, 22 Jul 2024 16:34:03 +0100 Subject: [PATCH] - replaced unsafe function StringUtils_createStringFromBufferInBuffer with function with length check to not exceed target buffer (LIB61850-447) --- src/common/inc/string_utilities.h | 3 ++ src/common/string_utilities.c | 12 ++++++ src/iec61850/server/mms_mapping/mms_mapping.c | 22 +++++----- src/mms/iso_mms/client/mms_client_identify.c | 6 +-- .../server/mms_named_variable_list_service.c | 40 +++++++++---------- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/common/inc/string_utilities.h b/src/common/inc/string_utilities.h index 404aec3e..c537d7e8 100644 --- a/src/common/inc/string_utilities.h +++ b/src/common/inc/string_utilities.h @@ -76,6 +76,9 @@ StringUtils_createStringFromBuffer(const uint8_t* buf, int size); LIB61850_INTERNAL char* StringUtils_createStringFromBufferInBuffer(char* newString, const uint8_t* buf, int size); +LIB61850_INTERNAL char* +StringUtils_createStringFromBufferInBufferMax(char* newString, const uint8_t* buf, int size, int maxBufSize); + LIB61850_INTERNAL void StringUtils_replace(char* string, char oldChar, char newChar); diff --git a/src/common/string_utilities.c b/src/common/string_utilities.c index 1608f964..aa447458 100644 --- a/src/common/string_utilities.c +++ b/src/common/string_utilities.c @@ -109,6 +109,18 @@ StringUtils_createStringFromBufferInBuffer(char* newString, const uint8_t* buf, return newString; } +char* +StringUtils_createStringFromBufferInBufferMax(char* newString, const uint8_t* buf, int size, int maxBufSize) +{ + if (size >= maxBufSize) + size = maxBufSize - 1; + + memcpy(newString, buf, size); + newString[size] = 0; + + return newString; +} + char* StringUtils_createStringInBuffer(char* newStr, int bufSize, int count, ...) { diff --git a/src/iec61850/server/mms_mapping/mms_mapping.c b/src/iec61850/server/mms_mapping/mms_mapping.c index 1435df2d..d6227ec7 100644 --- a/src/iec61850/server/mms_mapping/mms_mapping.c +++ b/src/iec61850/server/mms_mapping/mms_mapping.c @@ -3394,8 +3394,9 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom char* separator = strchr(variableId, '$'); - if (separator) { - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) variableId, separator - variableId); + if (separator) + { + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) variableId, separator - variableId, sizeof(str)); ln = LogicalDevice_getLogicalNode(ld, str); @@ -3426,8 +3427,9 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom char* separator = strchr(variableId, '$'); - if (separator) { - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) variableId, separator - variableId); + if (separator) + { + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) variableId, separator - variableId, sizeof(str)); ln = LogicalDevice_getLogicalNode(ld, str); @@ -3467,7 +3469,7 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom char str[65]; char subObjectBuf[65]; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) variableId, separator - variableId); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) variableId, separator - variableId, sizeof(str)); LogicalNode* ln = LogicalDevice_getLogicalNode(ld, str); @@ -3484,7 +3486,7 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom else { doEnd--; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) (doStart + 1), doEnd - doStart); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) (doStart + 1), doEnd - doStart, sizeof(str)); subObjectName = StringUtils_copyStringToBufferAndReplace(doEnd + 2, subObjectBuf, '$', '.'); } @@ -3533,7 +3535,7 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom char* subObjectName = NULL; char subObjectBuf[65]; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) variableId, separator - variableId); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) variableId, separator - variableId, sizeof(str)); LogicalNode* ln = LogicalDevice_getLogicalNode(ld, str); @@ -3551,7 +3553,7 @@ mmsListObjectsAccessHandler(void* parameter, MmsGetNameListType listType, MmsDom else { doEnd--; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) (doStart + 1), doEnd - doStart); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) (doStart + 1), doEnd - doStart, sizeof(str)); subObjectName = StringUtils_copyStringToBufferAndReplace(doEnd + 2, subObjectBuf, '$', '.'); } @@ -3684,7 +3686,7 @@ mmsReadAccessHandler (void* parameter, MmsDomain* domain, char* variableId, MmsS { char str[65]; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) variableId, separator - variableId); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) variableId, separator - variableId, sizeof(str)); LogicalNode* ln = LogicalDevice_getLogicalNode(ld, str); @@ -3704,7 +3706,7 @@ mmsReadAccessHandler (void* parameter, MmsDomain* domain, char* variableId, MmsS { doEnd--; - StringUtils_createStringFromBufferInBuffer(str, (uint8_t*) (doStart + 1), doEnd - doStart); + StringUtils_createStringFromBufferInBufferMax(str, (uint8_t*) (doStart + 1), doEnd - doStart, sizeof(str)); } if (fc == IEC61850_FC_SP) diff --git a/src/mms/iso_mms/client/mms_client_identify.c b/src/mms/iso_mms/client/mms_client_identify.c index 831b439d..c679a423 100644 --- a/src/mms/iso_mms/client/mms_client_identify.c +++ b/src/mms/iso_mms/client/mms_client_identify.c @@ -84,15 +84,15 @@ mmsClient_parseIdentifyResponse(MmsConnection self, ByteBuffer* response, uint32 switch (tag) { case 0x80: /* vendorName */ - vendorName = StringUtils_createStringFromBufferInBuffer(vendorNameBuf, buffer + bufPos, length); + vendorName = StringUtils_createStringFromBufferInBufferMax(vendorNameBuf, buffer + bufPos, length, sizeof(vendorNameBuf)); bufPos += length; break; case 0x81: /* modelName */ - modelName = StringUtils_createStringFromBufferInBuffer(modelNameBuf, buffer + bufPos, length); + modelName = StringUtils_createStringFromBufferInBufferMax(modelNameBuf, buffer + bufPos, length, sizeof(modelNameBuf)); bufPos += length; break; case 0x82: /* revision */ - revision = StringUtils_createStringFromBufferInBuffer(revisionBuf, buffer + bufPos, length); + revision = StringUtils_createStringFromBufferInBufferMax(revisionBuf, buffer + bufPos, length, sizeof (revisionBuf)); bufPos += length; break; case 0x83: /* list of abstract syntaxes */ diff --git a/src/mms/iso_mms/server/mms_named_variable_list_service.c b/src/mms/iso_mms/server/mms_named_variable_list_service.c index a38cfd7f..f9cdbacf 100644 --- a/src/mms/iso_mms/server/mms_named_variable_list_service.c +++ b/src/mms/iso_mms/server/mms_named_variable_list_service.c @@ -401,13 +401,13 @@ createNamedVariableList(MmsServer server, MmsDomain* domain, MmsDevice* device, char variableName[65]; char domainId[65]; - StringUtils_createStringFromBufferInBuffer(variableName, + StringUtils_createStringFromBufferInBufferMax(variableName, varSpec->choice.name.choice.domainspecific.itemId.buf, - varSpec->choice.name.choice.domainspecific.itemId.size); + varSpec->choice.name.choice.domainspecific.itemId.size, sizeof(variableName)); - StringUtils_createStringFromBufferInBuffer(domainId, + StringUtils_createStringFromBufferInBufferMax(domainId, varSpec->choice.name.choice.domainspecific.domainId.buf, - varSpec->choice.name.choice.domainspecific.domainId.size); + varSpec->choice.name.choice.domainspecific.domainId.size, sizeof(domainId)); MmsDomain* elementDomain = MmsDevice_getDomain(device, domainId); @@ -494,9 +494,9 @@ mmsServer_handleDefineNamedVariableListRequest( goto exit_free_struct; } - StringUtils_createStringFromBufferInBuffer(domainName, + StringUtils_createStringFromBufferInBufferMax(domainName, request->variableListName.choice.domainspecific.domainId.buf, - request->variableListName.choice.domainspecific.domainId.size); + request->variableListName.choice.domainspecific.domainId.size, sizeof(domainName)); MmsDomain* domain = MmsDevice_getDomain(device, domainName); @@ -517,9 +517,9 @@ mmsServer_handleDefineNamedVariableListRequest( goto exit_free_struct; } - StringUtils_createStringFromBufferInBuffer(variableListName, + StringUtils_createStringFromBufferInBufferMax(variableListName, request->variableListName.choice.domainspecific.itemId.buf, - request->variableListName.choice.domainspecific.itemId.size); + request->variableListName.choice.domainspecific.itemId.size, sizeof(variableListName)); if (MmsDomain_getNamedVariableList(domain, variableListName) != NULL) { mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_DEFINITION_OBJECT_EXISTS); @@ -567,9 +567,9 @@ mmsServer_handleDefineNamedVariableListRequest( goto exit_free_struct; } - StringUtils_createStringFromBufferInBuffer(variableListName, + StringUtils_createStringFromBufferInBufferMax(variableListName, request->variableListName.choice.aaspecific.buf, - request->variableListName.choice.aaspecific.size); + request->variableListName.choice.aaspecific.size, sizeof(variableListName)); if (MmsServerConnection_getNamedVariableList(connection, variableListName) != NULL) { mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_DEFINITION_OBJECT_EXISTS); @@ -611,9 +611,9 @@ mmsServer_handleDefineNamedVariableListRequest( goto exit_free_struct; } - StringUtils_createStringFromBufferInBuffer(variableListName, + StringUtils_createStringFromBufferInBufferMax(variableListName, request->variableListName.choice.vmdspecific.buf, - request->variableListName.choice.vmdspecific.size); + request->variableListName.choice.vmdspecific.size, sizeof(variableListName)); if (mmsServer_getNamedVariableListWithName(MmsDevice_getNamedVariableLists(connection->server->device), variableListName) != NULL) { mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_DEFINITION_OBJECT_EXISTS); @@ -758,11 +758,11 @@ mmsServer_handleGetNamedVariableListAttributesRequest( goto exit_function; } - StringUtils_createStringFromBufferInBuffer(domainName, request->choice.domainspecific.domainId.buf, - request->choice.domainspecific.domainId.size); + StringUtils_createStringFromBufferInBufferMax(domainName, request->choice.domainspecific.domainId.buf, + request->choice.domainspecific.domainId.size, sizeof(domainName)); - StringUtils_createStringFromBufferInBuffer(itemName, request->choice.domainspecific.itemId.buf, - request->choice.domainspecific.itemId.size); + StringUtils_createStringFromBufferInBufferMax(itemName, request->choice.domainspecific.itemId.buf, + request->choice.domainspecific.itemId.size, sizeof(itemName)); MmsDevice* mmsDevice = MmsServer_getDevice(connection->server); @@ -809,8 +809,8 @@ mmsServer_handleGetNamedVariableListAttributesRequest( goto exit_function; } - StringUtils_createStringFromBufferInBuffer(listName, request->choice.aaspecific.buf, - request->choice.aaspecific.size); + StringUtils_createStringFromBufferInBufferMax(listName, request->choice.aaspecific.buf, + request->choice.aaspecific.size, sizeof(listName)); MmsNamedVariableList varList = MmsServerConnection_getNamedVariableList(connection, listName); @@ -847,8 +847,8 @@ mmsServer_handleGetNamedVariableListAttributesRequest( goto exit_function; } - StringUtils_createStringFromBufferInBuffer(listName, request->choice.vmdspecific.buf, - request->choice.vmdspecific.size); + StringUtils_createStringFromBufferInBufferMax(listName, request->choice.vmdspecific.buf, + request->choice.vmdspecific.size, sizeof(listName)); MmsDevice* mmsDevice = MmsServer_getDevice(connection->server);