From 233cd20fd01c9cba4828c6b5bfd5656bc9534d54 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Wed, 23 Jan 2019 12:13:17 +0100 Subject: [PATCH] - MMS client: improved handling of malformed messages when receiving reports - MMS client: fixed potential memory leak when receiving malformed messages --- src/iec61850/client/client_report.c | 18 +++++++- src/iec61850/client/ied_connection.c | 46 ++++++++++--------- .../client/mms_client_get_var_access.c | 24 +++++----- src/mms/iso_mms/common/mms_value.c | 3 ++ 4 files changed, 56 insertions(+), 35 deletions(-) diff --git a/src/iec61850/client/client_report.c b/src/iec61850/client/client_report.c index f8139357..ec009e5f 100644 --- a/src/iec61850/client/client_report.c +++ b/src/iec61850/client/client_report.c @@ -43,6 +43,8 @@ struct sClientReport char* dataSetName; int dataSetNameSize; /* size of the dataSetName buffer */ + int dataSetSize; + MmsValue* entryId; MmsValue* dataReferences; MmsValue* dataSetValues; @@ -93,6 +95,8 @@ ClientReport_create() { ClientReport self = (ClientReport) GLOBAL_CALLOC(1, sizeof(struct sClientReport)); + self->dataSetSize = -1; + return self; } @@ -607,6 +611,18 @@ private_IedConnection_handleReport(IedConnection self, MmsValue* value) int dataSetSize = MmsValue_getBitStringSize(inclusion); + if (matchingReport->dataSetSize == -1) { + matchingReport->dataSetSize = dataSetSize; + } + else { + if (dataSetSize != matchingReport->dataSetSize) { + if (DEBUG_IED_CLIENT) + printf("IED_CLIENT: received malformed report (inclusion has no plausible size)\n"); + + goto exit_function; + } + } + int includedElements = MmsValue_getNumberOfSetBits(inclusion); if (DEBUG_IED_CLIENT) @@ -692,7 +708,7 @@ private_IedConnection_handleReport(IedConnection self, MmsValue* value) MmsValue_update(dataSetElement, newElementValue); if (DEBUG_IED_CLIENT) - printf("IED_CLIENT: update element value type: %i\n", MmsValue_getType(newElementValue)); + printf("IED_CLIENT: update element value type: %i\n", MmsValue_getType(newElementValue)); valueIndex++; diff --git a/src/iec61850/client/ied_connection.c b/src/iec61850/client/ied_connection.c index 04593f9f..12195040 100644 --- a/src/iec61850/client/ied_connection.c +++ b/src/iec61850/client/ied_connection.c @@ -437,39 +437,41 @@ informationReportHandler(void* parameter, char* domainName, if (DEBUG_IED_CLIENT) printf("DEBUG_IED_CLIENT: received information report for %s\n", variableListName); - if (domainName == NULL) { + if (value) { + if (domainName == NULL) { - if (isVariableListName) { - private_IedConnection_handleReport(self, value); - } - else { - if (strcmp(variableListName, "LastApplError") == 0) - handleLastApplErrorMessage(self, value); + if (isVariableListName) { + private_IedConnection_handleReport(self, value); + } else { - if (DEBUG_IED_CLIENT) - printf("IED_CLIENT: Received unknown variable list report for list: %s\n", variableListName); + if (strcmp(variableListName, "LastApplError") == 0) + handleLastApplErrorMessage(self, value); + else { + if (DEBUG_IED_CLIENT) + printf("IED_CLIENT: Received unknown variable list report for list: %s\n", variableListName); + } } } - } - else { - if (DEBUG_IED_CLIENT) - printf("IED_CLIENT: RCVD CommandTermination for %s/%s\n", domainName, variableListName); + else { + if (DEBUG_IED_CLIENT) + printf("IED_CLIENT: RCVD CommandTermination for %s/%s\n", domainName, variableListName); - LinkedList control = LinkedList_getNext(self->clientControls); + LinkedList control = LinkedList_getNext(self->clientControls); - while (control != NULL) { - ControlObjectClient object = (ControlObjectClient) control->data; + while (control != NULL) { + ControlObjectClient object = (ControlObjectClient) control->data; - char* objectRef = ControlObjectClient_getObjectReference(object); + char* objectRef = ControlObjectClient_getObjectReference(object); - if (doesReportMatchControlObject(domainName, variableListName, objectRef)) - private_ControlObjectClient_invokeCommandTerminationHandler(object); + if (doesReportMatchControlObject(domainName, variableListName, objectRef)) + private_ControlObjectClient_invokeCommandTerminationHandler(object); - control = LinkedList_getNext(control); + control = LinkedList_getNext(control); + } } - } - MmsValue_delete(value); + MmsValue_delete(value); + } } static IedConnection diff --git a/src/mms/iso_mms/client/mms_client_get_var_access.c b/src/mms/iso_mms/client/mms_client_get_var_access.c index d66f5f22..9da333a2 100644 --- a/src/mms/iso_mms/client/mms_client_get_var_access.c +++ b/src/mms/iso_mms/client/mms_client_get_var_access.c @@ -138,23 +138,23 @@ mmsClient_parseGetVariableAccessAttributesResponse(ByteBuffer* message, uint32_t asn_dec_rval_t rval = ber_decode(NULL, &asn_DEF_MmsPdu, (void**) &mmsPdu, ByteBuffer_getBuffer(message), ByteBuffer_getSize(message)); - if (rval.code != RC_OK) - return NULL; + if (rval.code == RC_OK) { - if (mmsPdu->present == MmsPdu_PR_confirmedResponsePdu) { + if (mmsPdu->present == MmsPdu_PR_confirmedResponsePdu) { - if (invokeId != NULL) - *invokeId = mmsClient_getInvokeId(&mmsPdu->choice.confirmedResponsePdu); + if (invokeId != NULL) + *invokeId = mmsClient_getInvokeId(&mmsPdu->choice.confirmedResponsePdu); - if (mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.present == - ConfirmedServiceResponse_PR_getVariableAccessAttributes) - { - GetVariableAccessAttributesResponse_t* response; + if (mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.present == + ConfirmedServiceResponse_PR_getVariableAccessAttributes) + { + GetVariableAccessAttributesResponse_t* response; - response = &(mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.choice.getVariableAccessAttributes); - TypeSpecification_t* asnTypeSpec = &response->typeSpecification; + response = &(mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.choice.getVariableAccessAttributes); + TypeSpecification_t* asnTypeSpec = &response->typeSpecification; - typeSpec = createTypeSpecification(asnTypeSpec); + typeSpec = createTypeSpecification(asnTypeSpec); + } } } diff --git a/src/mms/iso_mms/common/mms_value.c b/src/mms/iso_mms/common/mms_value.c index b6ae3430..f2453907 100644 --- a/src/mms/iso_mms/common/mms_value.c +++ b/src/mms/iso_mms/common/mms_value.c @@ -1190,6 +1190,9 @@ MmsValue_deleteIfNotNull(MmsValue* self) void MmsValue_delete(MmsValue* self) { + if (self == NULL) + return; + switch (self->type) { case MMS_INTEGER: