From 0d4930ac25d67bf9dc05db515925024aa799d2d4 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Sat, 28 Mar 2020 12:47:56 +0100 Subject: [PATCH] - MMS server: fixed potential crash when get-named-variable-list-attributes response doesn't fit in MMS PDU. Server returns service error in this case --- .../client_example_reporting.c | 42 ++++++++++++------- src/common/byte_buffer.c | 3 +- src/mms/iso_mms/client/mms_client_common.c | 10 ++++- .../iso_mms/client/mms_client_connection.c | 17 +++++--- src/mms/iso_mms/client/mms_client_write.c | 1 - .../server/mms_named_variable_list_service.c | 23 ++++++++-- src/mms/iso_mms/server/mms_server_common.c | 12 +++++- 7 files changed, 79 insertions(+), 29 deletions(-) diff --git a/examples/iec61850_client_example_reporting/client_example_reporting.c b/examples/iec61850_client_example_reporting/client_example_reporting.c index f070248a..29805ae3 100644 --- a/examples/iec61850_client_example_reporting/client_example_reporting.c +++ b/examples/iec61850_client_example_reporting/client_example_reporting.c @@ -43,19 +43,33 @@ reportCallbackFunction(void* parameter, ClientReport report) printf(" report contains timestamp (%u): %s", (unsigned int) unixTime, timeBuf); } - int i; - for (i = 0; i < LinkedList_size(dataSetDirectory); i++) { - ReasonForInclusion reason = ClientReport_getReasonForInclusion(report, i); + if (dataSetDirectory) { + int i; + for (i = 0; i < LinkedList_size(dataSetDirectory); i++) { + ReasonForInclusion reason = ClientReport_getReasonForInclusion(report, i); - if (reason != IEC61850_REASON_NOT_INCLUDED) { + if (reason != IEC61850_REASON_NOT_INCLUDED) { - LinkedList entry = LinkedList_get(dataSetDirectory, i); + char valBuffer[500]; + sprintf(valBuffer, "no value"); - char* entryName = (char*) entry->data; + if (dataSetValues) { + MmsValue* value = MmsValue_getElement(dataSetValues, i); - printf(" %s (included for reason %i)\n", entryName, reason); + if (value) { + MmsValue_printToBuffer(value, valBuffer, 500); + } + } + + LinkedList entry = LinkedList_get(dataSetDirectory, i); + + char* entryName = (char*) entry->data; + + printf(" %s (included for reason %i): %s\n", entryName, reason, valBuffer); + } } } + } int @@ -89,23 +103,23 @@ main(int argc, char** argv) LinkedList dataSetDirectory = NULL; /* read data set directory */ - dataSetDirectory = IedConnection_getDataSetDirectory(con, &error, "simpleIOGenericIO/LLN0.Events", NULL); + dataSetDirectory = IedConnection_getDataSetDirectory(con, &error, "testmodelSENSORS/LLN0.DataSetST_Attr", NULL); if (error != IED_ERROR_OK) { printf("Reading data set directory failed!\n"); - goto exit_error; + // goto exit_error; } /* read data set */ - clientDataSet = IedConnection_readDataSetValues(con, &error, "simpleIOGenericIO/LLN0.Events", NULL); + clientDataSet = IedConnection_readDataSetValues(con, &error, "testmodelSENSORS/LLN0.DataSetST_Attr", NULL); if (clientDataSet == NULL) { printf("failed to read dataset\n"); - goto exit_error; + // goto exit_error; } /* Read RCB values */ - rcb = IedConnection_getRCBValues(con, &error, "simpleIOGenericIO/LLN0.RP.EventsRCB01", NULL); + rcb = IedConnection_getRCBValues(con, &error, "testmodelSENSORS/LLN0.RP.events01", NULL); if (error != IED_ERROR_OK) { printf("getRCBValues service error!\n"); @@ -115,12 +129,12 @@ main(int argc, char** argv) /* prepare the parameters of the RCP */ ClientReportControlBlock_setResv(rcb, true); ClientReportControlBlock_setTrgOps(rcb, TRG_OPT_DATA_CHANGED | TRG_OPT_QUALITY_CHANGED | TRG_OPT_GI); - ClientReportControlBlock_setDataSetReference(rcb, "simpleIOGenericIO/LLN0$Events"); /* NOTE the "$" instead of "." ! */ + ClientReportControlBlock_setDataSetReference(rcb, "testmodelSENSORS/LLN0$DataSetST_Attr"); /* NOTE the "$" instead of "." ! */ ClientReportControlBlock_setRptEna(rcb, true); ClientReportControlBlock_setGI(rcb, true); /* Configure the report receiver */ - IedConnection_installReportHandler(con, "simpleIOGenericIO/LLN0.RP.EventsRCB", ClientReportControlBlock_getRptId(rcb), reportCallbackFunction, + IedConnection_installReportHandler(con, "testmodelSENSORS/LLN0.events01", ClientReportControlBlock_getRptId(rcb), reportCallbackFunction, (void*) dataSetDirectory); /* Write RCB parameters and enable report */ diff --git a/src/common/byte_buffer.c b/src/common/byte_buffer.c index 9c1a4f9d..b3747d11 100644 --- a/src/common/byte_buffer.c +++ b/src/common/byte_buffer.c @@ -61,8 +61,9 @@ ByteBuffer_append(ByteBuffer* self, uint8_t* data, int dataSize) self->size += dataSize; return dataSize; } - else + else { return -1; + } } int diff --git a/src/mms/iso_mms/client/mms_client_common.c b/src/mms/iso_mms/client/mms_client_common.c index 468cd00c..1b5e1972 100644 --- a/src/mms/iso_mms/client/mms_client_common.c +++ b/src/mms/iso_mms/client/mms_client_common.c @@ -33,7 +33,15 @@ int mmsClient_write_out(void *buffer, size_t size, void *app_key) { ByteBuffer* writeBuffer = (ByteBuffer*) app_key; - return ByteBuffer_append(writeBuffer, (uint8_t*) buffer, size); + + int appendedBytes = ByteBuffer_append(writeBuffer, (uint8_t*) buffer, size); + + if (appendedBytes == -1) { + if (DEBUG_MMS_CLIENT) + printf("MMS_CLIENT: message exceeds maximum PDU size!\n"); + } + + return appendedBytes; } diff --git a/src/mms/iso_mms/client/mms_client_connection.c b/src/mms/iso_mms/client/mms_client_connection.c index fe41e3bf..8dff7dfc 100644 --- a/src/mms/iso_mms/client/mms_client_connection.c +++ b/src/mms/iso_mms/client/mms_client_connection.c @@ -4334,15 +4334,20 @@ MmsConnection_writeMultipleVariablesAsync(MmsConnection self, MmsError* mmsError invokeId = getNextInvokeId(self); - mmsClient_createWriteMultipleItemsRequest(invokeId, domainId, items, values, payload); + if (mmsClient_createWriteMultipleItemsRequest(invokeId, domainId, items, values, payload) != -1) { + MmsClientInternalParameter intParam; + intParam.ptr = NULL; - MmsClientInternalParameter intParam; - intParam.ptr = NULL; + MmsError err = sendAsyncRequest(self, invokeId, payload, MMS_CALL_TYPE_WRITE_MULTIPLE_VARIABLES, handler, parameter, intParam); - MmsError err = sendAsyncRequest(self, invokeId, payload, MMS_CALL_TYPE_WRITE_MULTIPLE_VARIABLES, handler, parameter, intParam); + if (mmsError) + *mmsError = err; + } + else { + *mmsError = MMS_ERROR_RESOURCE_OTHER; + return 0; + } - if (mmsError) - *mmsError = err; exit_function: return invokeId; diff --git a/src/mms/iso_mms/client/mms_client_write.c b/src/mms/iso_mms/client/mms_client_write.c index ec88c8a6..e1e62239 100644 --- a/src/mms/iso_mms/client/mms_client_write.c +++ b/src/mms/iso_mms/client/mms_client_write.c @@ -319,7 +319,6 @@ mmsClient_createWriteMultipleItemsRequest(uint32_t invokeId, const char* domainI asn_DEF_MmsPdu.free_struct(&asn_DEF_MmsPdu, mmsPdu, 0); return rval.encoded; - } int 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 5e4f0704..89634181 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 @@ -601,7 +601,7 @@ exit_free_struct: #if (MMS_GET_DATA_SET_ATTRIBUTES == 1) -static void +static bool createGetNamedVariableListAttributesResponse(int invokeId, ByteBuffer* response, MmsNamedVariableList variableList) { @@ -658,9 +658,14 @@ createGetNamedVariableListAttributesResponse(int invokeId, ByteBuffer* response, variable = LinkedList_getNext(variable); } - der_encode(&asn_DEF_MmsPdu, mmsPdu, mmsServer_write_out, (void*) response); + asn_enc_rval_t res = der_encode(&asn_DEF_MmsPdu, mmsPdu, mmsServer_write_out, (void*) response); asn_DEF_MmsPdu.free_struct(&asn_DEF_MmsPdu, mmsPdu, 0); + + if (res.encoded == -1) + return false; + else + return true; } void @@ -705,8 +710,18 @@ mmsServer_handleGetNamedVariableListAttributesRequest( MmsNamedVariableList variableList = MmsDomain_getNamedVariableList(domain, itemName); - if (variableList != NULL) - createGetNamedVariableListAttributesResponse(invokeId, response, variableList); + if (variableList != NULL) { + + int bufSize = ByteBuffer_getSize(response); + + if (createGetNamedVariableListAttributesResponse(invokeId, response, variableList) == false) { + + /* encoding failed - probably because buffer size is too small for message */ + ByteBuffer_setSize(response, 0); + + mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_RESOURCE_OTHER); + } + } else mmsMsg_createServiceErrorPdu(invokeId, response, MMS_ERROR_ACCESS_OBJECT_NON_EXISTENT); } diff --git a/src/mms/iso_mms/server/mms_server_common.c b/src/mms/iso_mms/server/mms_server_common.c index 96709b6c..b28bcce9 100644 --- a/src/mms/iso_mms/server/mms_server_common.c +++ b/src/mms/iso_mms/server/mms_server_common.c @@ -1,7 +1,7 @@ /* * mms_server_common.c * - * Copyright 2013-2016 Michael Zillgith + * Copyright 2013-2020 Michael Zillgith * * This file is part of libIEC61850. * @@ -29,7 +29,15 @@ int mmsServer_write_out(const void *buffer, size_t size, void *app_key) { ByteBuffer* writeBuffer = (ByteBuffer*) app_key; - return ByteBuffer_append(writeBuffer, (uint8_t*) buffer, size); + + int appendedBytes = ByteBuffer_append(writeBuffer, (uint8_t*) buffer, size); + + if (appendedBytes == -1) { + if (DEBUG_MMS_SERVER) + printf("MMS_SERVER: message exceeds maximum PDU size!\n"); + } + + return appendedBytes; } MmsPdu_t*