- MMS client: improved handling of malformed messages when reading data

- 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

@@ -3,7 +3,7 @@
  *
  *  Client implementation for IEC 61850 reporting.
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -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 @@ 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 @@ 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++;
 

src/iec61850/client/ied_connection.c

@@ -1,7 +1,7 @@
 /*
  *  ied_connection.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -96,6 +96,9 @@ iedConnection_mapMmsErrorToIedError(MmsError mmsError)
     case MMS_ERROR_ACCESS_OBJECT_VALUE_INVALID:
     	return IED_ERROR_OBJECT_VALUE_INVALID;
 
+    case MMS_ERROR_PARSING_RESPONSE:
+        return IED_ERROR_MALFORMED_MESSAGE;
+
     default:
         return IED_ERROR_UNKNOWN;
     }
@@ -484,42 +487,48 @@ informationReportHandler(void* parameter, char* domainName,
 {
     IedConnection self = (IedConnection) parameter;
 
-    if (DEBUG_IED_CLIENT)
-        printf("DEBUG_IED_CLIENT: received information report for %s\n", variableListName);
+    if (value) {
+        if (DEBUG_IED_CLIENT)
+            printf("IED_CLIENT: received information report for %s\n", variableListName);
 
-    if (domainName == NULL) {
+        if (domainName == NULL) {
 
-        if (isVariableListName) {
-            iedConnection_handleReport(self, value);
-        }
-        else {
-            if (strcmp(variableListName, "LastApplError") == 0)
-                handleLastApplErrorMessage(self, value);
+            if (isVariableListName) {
+                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;
 
-           const char* objectRef = ControlObjectClient_getObjectReference(object);
+               const char* objectRef = ControlObjectClient_getObjectReference(object);
 
-           if (doesReportMatchControlObject(domainName, variableListName, objectRef))
-               controlObjectClient_invokeCommandTerminationHandler(object);
+               if (doesReportMatchControlObject(domainName, variableListName, objectRef))
+                   controlObjectClient_invokeCommandTerminationHandler(object);
 
-           control = LinkedList_getNext(control);
+               control = LinkedList_getNext(control);
+            }
         }
-    }
 
-    MmsValue_delete(value);
+        MmsValue_delete(value);
+    }
+    else {
+        if (DEBUG_IED_CLIENT)
+             printf("IED_CLIENT: report for %s/%s: value invalid\n", domainName, variableListName);
+    }
 }
 
 static void

src/iec61850/inc/iec61850_client.h

@@ -1,7 +1,7 @@
 /*
  *  iec61850_client.h
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -159,6 +159,9 @@ typedef enum {
     /** The object is invalidated (returned by server) */
     IED_ERROR_OBJECT_INVALIDATED = 33,
 
+    /** Received an invalid response message from the server */
+    IED_ERROR_MALFORMED_MESSAGE = 34,
+
     /** Service not implemented */
     IED_ERROR_SERVICE_NOT_IMPLEMENTED = 98,
 

src/iec61850/server/mms_mapping/control.c

@@ -1,7 +1,7 @@
 /*
  *  control.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -316,10 +316,25 @@ ControlObject_create(IedServer iedServer, MmsDomain* domain, char* lnName, char*
     self->iedServer = iedServer;
 
     MmsVariableSpecification* ctlValSpec = MmsVariableSpecification_getChildSpecificationByName(operSpec, "ctlVal", NULL);
-    self->ctlVal = MmsValue_newDefaultValue(ctlValSpec);
+
+    if (ctlValSpec) {
+        self->ctlVal = MmsValue_newDefaultValue(ctlValSpec);
+    }
+    else {
+        if (DEBUG_IED_SERVER)
+            printf("IED_SERVER: control object %s/%s.%s has no ctlVal element!\n", domain->domainName, lnName, name);
+    }
+
 
     MmsVariableSpecification* originSpec = MmsVariableSpecification_getChildSpecificationByName(operSpec, "origin", NULL);
-    self->origin = MmsValue_newDefaultValue(originSpec);
+
+    if (originSpec) {
+        self->origin = MmsValue_newDefaultValue(originSpec);
+    }
+    else {
+        if (DEBUG_IED_SERVER)
+            printf("IED_SERVER: control object %s/%s.%s has no origin element!\n", domain->domainName, lnName, name);
+    }
 
     self->ctlNum = MmsValue_newUnsigned(8);
 

src/mms/iso_mms/client/mms_client_connection.c

@@ -1,7 +1,7 @@
 /*
  *  mms_client_connection.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -657,7 +657,10 @@ handleAsyncResponse(MmsConnection self, ByteBuffer* response, uint32_t bufPos, M
             if (response) {
                 MmsValue* value = mmsClient_parseReadResponse(response, NULL, false);
 
-                handler(outstandingCall->invokeId, outstandingCall->userParameter, MMS_ERROR_NONE, value);
+                if (value == NULL)
+                    err = MMS_ERROR_PARSING_RESPONSE;
+
+                handler(outstandingCall->invokeId, outstandingCall->userParameter, err, value);
             }
 
         }
@@ -673,7 +676,10 @@ handleAsyncResponse(MmsConnection self, ByteBuffer* response, uint32_t bufPos, M
             if (response) {
                 MmsValue* value = mmsClient_parseReadResponse(response, NULL, true);
 
-                handler(outstandingCall->invokeId, outstandingCall->userParameter, MMS_ERROR_NONE, value);
+                if (value == NULL)
+                    err = MMS_ERROR_PARSING_RESPONSE;
+
+                handler(outstandingCall->invokeId, outstandingCall->userParameter, err, value);
             }
 
         }
@@ -726,7 +732,7 @@ handleAsyncResponse(MmsConnection self, ByteBuffer* response, uint32_t bufPos, M
 
                 LinkedList accessSpec = mmsClient_parseGetNamedVariableListAttributesResponse(response, &deletable);
 
-                if (accessSpec == false)
+                if (accessSpec == NULL)
                     err = MMS_ERROR_PARSING_RESPONSE;
 
                 handler(outstandingCall->invokeId, outstandingCall->userParameter, err, accessSpec, deletable);
@@ -779,6 +785,9 @@ handleAsyncResponse(MmsConnection self, ByteBuffer* response, uint32_t bufPos, M
         else {
             MmsVariableSpecification* typeSpec = mmsClient_parseGetVariableAccessAttributesResponse(response, NULL);
 
+            if (typeSpec == NULL)
+                err = MMS_ERROR_PARSING_RESPONSE;
+
             handler(outstandingCall->invokeId, outstandingCall->userParameter, err, typeSpec);
         }
     }

src/mms/iso_mms/client/mms_client_get_var_access.c

@@ -138,24 +138,25 @@ 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);
+            }
         }
+
     }
 
     asn_DEF_MmsPdu.free_struct(&asn_DEF_MmsPdu, mmsPdu, 0);

src/mms/iso_mms/client/mms_client_read.c

@@ -1,7 +1,7 @@
 /*
  *  mms_client_read.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -46,11 +46,14 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi
     int i = 0;
 
     for (i = 0; i < elementCount; i++) {
+
+        value = NULL;
+
         AccessResult_PR presentType = accessResultList[i]->present;
 
         if (presentType == AccessResult_PR_failure) {
             if (DEBUG_MMS_CLIENT)
-                printf("access error!\n");
+                printf("MMS CLIENT: received access error!\n");
 
             if (accessResultList[i]->choice.failure.size > 0) {
                 int errorCode = (int) accessResultList[i]->choice.failure.buf[0];
@@ -66,72 +69,138 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi
                 value = MmsValue_newDataAccessError(DATA_ACCESS_ERROR_UNKNOWN);
         }
         else if (presentType == AccessResult_PR_array) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            value->type = MMS_ARRAY;
 
             int arrayElementCount =
                     accessResultList[i]->choice.array.list.count;
 
-            value->value.structure.size = arrayElementCount;
-            value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(arrayElementCount, sizeof(MmsValue*));
-
-            int j;
-
-            for (j = 0; j < arrayElementCount; j++) {
-                value->value.structure.components[j] = mmsMsg_parseDataElement(
-                        accessResultList[i]->choice.array.list.array[j]);
+            if (arrayElementCount > 0) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                value->type = MMS_ARRAY;
+                value->value.structure.size = arrayElementCount;
+                value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(arrayElementCount, sizeof(MmsValue*));
+
+                int j;
+
+                for (j = 0; j < arrayElementCount; j++) {
+                    value->value.structure.components[j] = mmsMsg_parseDataElement(
+                            accessResultList[i]->choice.array.list.array[j]);
+
+                    if (value->value.structure.components[j] == NULL) {
+                        MmsValue_delete(value);
+                        value = NULL;
+                        break;
+                    }
+                }
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid array size)!\n");
             }
+
         }
         else if (presentType == AccessResult_PR_structure) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            value->type = MMS_STRUCTURE;
 
             int componentCount =
                     accessResultList[i]->choice.structure.list.count;
 
-            value->value.structure.size = componentCount;
-            value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
-
-            int j;
-            for (j = 0; j < componentCount; j++) {
-                value->value.structure.components[j] = mmsMsg_parseDataElement(
-                        accessResultList[i]->choice.structure.list.array[j]);
+            if (componentCount > 0) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                value->type = MMS_STRUCTURE;
+                value->value.structure.size = componentCount;
+                value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
+
+                int j;
+                for (j = 0; j < componentCount; j++) {
+                    value->value.structure.components[j] = mmsMsg_parseDataElement(
+                            accessResultList[i]->choice.structure.list.array[j]);
+
+                    if (value->value.structure.components[j] == NULL) {
+                        MmsValue_delete(value);
+                        value = NULL;
+                        break;
+                    }
+                }
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid structure size)!\n");
             }
+
         }
         else if (presentType == AccessResult_PR_bitstring) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            value->type = MMS_BIT_STRING;
+
             int size = accessResultList[i]->choice.bitstring.size;
 
-            value->value.bitString.size = (size * 8)
-                    - accessResultList[i]->choice.bitstring.bits_unused;
+            if (size > 0) {
+
+                int maxSize = (size * 8);
+                int bitSize = maxSize - accessResultList[i]->choice.bitstring.bits_unused;
+
+                if ((bitSize > 0) && (maxSize >= bitSize)) {
+
+                    value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                    value->type = MMS_BIT_STRING;
 
-            value->value.bitString.buf = (uint8_t*) GLOBAL_MALLOC(size);
-            memcpy(value->value.bitString.buf,
-                    accessResultList[i]->choice.bitstring.buf, size);
+
+                    value->value.bitString.size = (size * 8)
+                            - accessResultList[i]->choice.bitstring.bits_unused;
+
+                    value->value.bitString.buf = (uint8_t*) GLOBAL_MALLOC(size);
+                    memcpy(value->value.bitString.buf,
+                            accessResultList[i]->choice.bitstring.buf, size);
+                }
+                else {
+                    if (DEBUG_MMS_CLIENT)
+                        printf("MMS CLIENT: error parsing access result (bit string padding problem)!\n");
+                }
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (bit string size 0 or negative)!\n");
+            }
 
         }
         else if (presentType == AccessResult_PR_integer) {
-            Asn1PrimitiveValue* berInteger =
-                    BerInteger_createFromBuffer(accessResultList[i]->choice.integer.buf,
-                            accessResultList[i]->choice.integer.size);
 
-            value = MmsValue_newIntegerFromBerInteger(berInteger);
+            int size = accessResultList[i]->choice.integer.size;
+
+            if (size > 0) {
+                Asn1PrimitiveValue* berInteger =
+                        BerInteger_createFromBuffer(accessResultList[i]->choice.integer.buf, size);
+
+                value = MmsValue_newIntegerFromBerInteger(berInteger);
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid integer size)!\n");
+            }
         }
         else if (presentType == AccessResult_PR_unsigned) {
-            Asn1PrimitiveValue* berInteger =
-                    BerInteger_createFromBuffer(accessResultList[i]->choice.Unsigned.buf,
-                            accessResultList[i]->choice.Unsigned.size);
 
-            value = MmsValue_newUnsignedFromBerInteger(berInteger);
+            int size = accessResultList[i]->choice.Unsigned.size;
+
+            if (size > 0) {
+                Asn1PrimitiveValue* berInteger =
+                        BerInteger_createFromBuffer(accessResultList[i]->choice.Unsigned.buf,
+                                accessResultList[i]->choice.Unsigned.size);
+
+                value = MmsValue_newUnsignedFromBerInteger(berInteger);
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid unsigned size)!\n");
+            }
+
         }
         else if (presentType == AccessResult_PR_floatingpoint) {
-            int size = accessResultList[i]->choice.floatingpoint.size;
 
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            value->type = MMS_FLOAT;
+            int size = accessResultList[i]->choice.floatingpoint.size;
 
             if (size == 5) { /* FLOAT32 */
+
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                value->type = MMS_FLOAT;
+
                 value->value.floatingPoint.formatWidth = 32;
                 value->value.floatingPoint.exponentWidth = accessResultList[i]->choice.floatingpoint.buf[0];
 
@@ -146,8 +215,11 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi
 #endif
 
             }
+            else if (size == 9) { /* FLOAT64 */
+
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                value->type = MMS_FLOAT;
 
-            if (size == 9) { /* FLOAT64 */
                 value->value.floatingPoint.formatWidth = 64;
                 value->value.floatingPoint.exponentWidth = accessResultList[i]->choice.floatingpoint.buf[0];
 
@@ -161,46 +233,70 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi
                 memcpy(value->value.floatingPoint.buf, floatBuf, 8);
 #endif
             }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing float (size must be 5 or 9, is %i)\n", size);
+            }
 
         }
         else if (presentType == AccessResult_PR_visiblestring) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-
-            value->type = MMS_VISIBLE_STRING;
 
             int strSize = accessResultList[i]->choice.visiblestring.size;
 
-            value->value.visibleString.buf = (char*) GLOBAL_MALLOC(strSize + 1);
-            value->value.visibleString.size = strSize;
+            if (strSize >= 0) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+
+                value->type = MMS_VISIBLE_STRING;
+                value->value.visibleString.buf = (char*) GLOBAL_MALLOC(strSize + 1);
+                value->value.visibleString.size = strSize;
 
-            memcpy(value->value.visibleString.buf,
-                    accessResultList[i]->choice.visiblestring.buf,
-                    strSize);
+                memcpy(value->value.visibleString.buf,
+                        accessResultList[i]->choice.visiblestring.buf,
+                        strSize);
+
+                value->value.visibleString.buf[strSize] = 0;
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid visible-string size)\n");
+            }
 
-            value->value.visibleString.buf[strSize] = 0;
         }
         else if (presentType == AccessResult_PR_mMSString) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-
-            value->type = MMS_STRING;
 
             int strSize = accessResultList[i]->choice.mMSString.size;
 
-            value->value.visibleString.buf = (char*) GLOBAL_MALLOC(strSize + 1);
-            value->value.visibleString.size = strSize;
+            if (strSize >= 0) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
 
-            memcpy(value->value.visibleString.buf,
-                    accessResultList[i]->choice.mMSString.buf, strSize);
+                value->type = MMS_STRING;
+                value->value.visibleString.buf = (char*) GLOBAL_MALLOC(strSize + 1);
+                value->value.visibleString.size = strSize;
 
-            value->value.visibleString.buf[strSize] = 0;
+                memcpy(value->value.visibleString.buf,
+                        accessResultList[i]->choice.mMSString.buf, strSize);
 
+                value->value.visibleString.buf[strSize] = 0;
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid mms-string size)\n");
+            }
         }
         else if (presentType == AccessResult_PR_utctime) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
 
-            value->type = MMS_UTC_TIME;
-            memcpy(value->value.utcTime,
-                    accessResultList[i]->choice.utctime.buf, 8);
+            int size = accessResultList[i]->choice.utctime.size;
+
+            if (size == 8) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+
+                value->type = MMS_UTC_TIME;
+                memcpy(value->value.utcTime, accessResultList[i]->choice.utctime.buf, 8);
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing UTC time (size is %i instead of 8\n", size);
+            }
         }
         else if (presentType == AccessResult_PR_boolean) {
             value = MmsValue_newBoolean(accessResultList[i]->choice.boolean);
@@ -208,25 +304,36 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi
         else if (presentType == AccessResult_PR_binarytime) {
             int size = accessResultList[i]->choice.binarytime.size;
 
-            if (size <= 6) {
+            if ((size == 4) || (size == 6)) {
                 value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
                 value->type = MMS_BINARY_TIME;
                 value->value.binaryTime.size = size;
                 memcpy(value->value.binaryTime.buf, accessResultList[i]->choice.binarytime.buf, size);
             }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing binary time (size must be 4 or 6, is %i\n", size);
+            }
         }
         else if (presentType == AccessResult_PR_octetstring) {
             int size = accessResultList[i]->choice.octetstring.size;
 
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            value->type = MMS_OCTET_STRING;
-            value->value.octetString.maxSize = size;
-            value->value.octetString.size = size;
-            value->value.octetString.buf = (uint8_t*) GLOBAL_MALLOC(size);
-            memcpy(value->value.octetString.buf, accessResultList[i]->choice.octetstring.buf, size);
+            if (size >= 0) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+                value->type = MMS_OCTET_STRING;
+                value->value.octetString.maxSize = size;
+                value->value.octetString.size = size;
+                value->value.octetString.buf = (uint8_t*) GLOBAL_MALLOC(size);
+                memcpy(value->value.octetString.buf, accessResultList[i]->choice.octetstring.buf, size);
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing access result (invalid octet-string size)\n");
+            }
         }
         else {
-            printf("unknown type %i\n", presentType);
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: unknown type %i in access result\n", presentType);
             value = MmsValue_newDataAccessError(DATA_ACCESS_ERROR_OBJECT_VALUE_INVALID);
         }
 
@@ -254,21 +361,20 @@ mmsClient_parseReadResponse(ByteBuffer* message, uint32_t* invokeId, bool create
     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 (mmsPdu->present == MmsPdu_PR_confirmedResponsePdu) {
+    if (rval.code == RC_OK) {
+        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_read) {
-            ReadResponse_t* response = &(mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.choice.read);
+            if (mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.present == ConfirmedServiceResponse_PR_read) {
+                ReadResponse_t* response = &(mmsPdu->choice.confirmedResponsePdu.confirmedServiceResponse.choice.read);
 
-            int elementCount = response->listOfAccessResult.list.count;
+                int elementCount = response->listOfAccessResult.list.count;
 
-            valueList = mmsClient_parseListOfAccessResults(response->listOfAccessResult.list.array,
-                    elementCount, createArray);
+                valueList = mmsClient_parseListOfAccessResults(response->listOfAccessResult.list.array,
+                        elementCount, createArray);
+            }
         }
     }
 

src/mms/iso_mms/common/mms_common_msg.c

@@ -1,7 +1,7 @@
 /*
  *  mms_common_msg.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -185,113 +185,192 @@ mmsMsg_parseDataElement(Data_t* dataElement)
 {
     MmsValue* value = NULL;
 
-    if (dataElement->present == Data_PR_structure) {
-        value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+    if (dataElement->present == Data_PR_array) {
 
-        int componentCount = dataElement->choice.structure->list.count;
+        int componentCount = dataElement->choice.array->list.count;
+
+        if (componentCount > 0) {
+            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
 
-        value->type = MMS_STRUCTURE;
-        value->value.structure.size = componentCount;
-        value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
+            value->type = MMS_ARRAY;
+            value->value.structure.size = componentCount;
+            value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
 
-        int i;
+            int i;
+
+            for (i = 0; i < componentCount; i++) {
+                value->value.structure.components[i] =
+                        mmsMsg_parseDataElement(dataElement->choice.array->list.array[i]);
 
-        for (i = 0; i < componentCount; i++) {
-            value->value.structure.components[i] =
-                    mmsMsg_parseDataElement(dataElement->choice.structure->list.array[i]);
+                if (value->value.structure.components[i] == NULL) {
+                    MmsValue_delete(value);
+                    value = NULL;
+                    break;
+                }
+            }
+        }
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing data element (invalid array size)!\n");
         }
+
     }
-    else if (dataElement->present == Data_PR_array) {
-        value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+    else if (dataElement->present == Data_PR_structure) {
 
-        int componentCount = dataElement->choice.array->list.count;
+        int componentCount = dataElement->choice.structure->list.count;
+
+        if (componentCount > 0) {
+            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+
+            value->type = MMS_STRUCTURE;
+            value->value.structure.size = componentCount;
+            value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
 
-        value->type = MMS_ARRAY;
-        value->value.structure.size = componentCount;
-        value->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*));
+            int i;
 
-        int i;
+            for (i = 0; i < componentCount; i++) {
+                value->value.structure.components[i] =
+                        mmsMsg_parseDataElement(dataElement->choice.structure->list.array[i]);
 
-        for (i = 0; i < componentCount; i++) {
-            value->value.structure.components[i] =
-                    mmsMsg_parseDataElement(dataElement->choice.array->list.array[i]);
+                if (value->value.structure.components[i] == NULL) {
+                    MmsValue_delete(value);
+                    value = NULL;
+                    break;
+                }
+            }
+        }
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing data element (invalid structure size)!\n");
         }
     }
-    else {
-        if (dataElement->present == Data_PR_integer) {
+    else if (dataElement->present == Data_PR_integer) {
+
+        if (dataElement->choice.integer.size > 0) {
             Asn1PrimitiveValue* berInteger = BerInteger_createFromBuffer(
                     dataElement->choice.integer.buf, dataElement->choice.integer.size);
 
             value = MmsValue_newIntegerFromBerInteger(berInteger);
         }
-        else if (dataElement->present == Data_PR_unsigned) {
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing data element (invalid integer size)!\n");
+        }
+    }
+    else if (dataElement->present == Data_PR_unsigned) {
+
+        if (dataElement->choice.Unsigned.size > 0) {
             Asn1PrimitiveValue* berInteger = BerInteger_createFromBuffer(
                     dataElement->choice.Unsigned.buf, dataElement->choice.Unsigned.size);
 
             value = MmsValue_newUnsignedFromBerInteger(berInteger);
         }
-        else if (dataElement->present == Data_PR_visiblestring) {
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing data element (invalid unsigned size)!\n");
+        }
+    }
+    else if (dataElement->present == Data_PR_visiblestring) {
+
+        if (dataElement->choice.visiblestring.size >= 0) {
             value = MmsValue_newVisibleStringFromByteArray(dataElement->choice.visiblestring.buf,
                     dataElement->choice.visiblestring.size);
         }
-        else if (dataElement->present == Data_PR_mMSString) {
+    }
+    else if (dataElement->present == Data_PR_mMSString) {
+
+        if ( dataElement->choice.mMSString.size >= 0) {
             value = MmsValue_newMmsStringFromByteArray(dataElement->choice.mMSString.buf,
                     dataElement->choice.mMSString.size);
         }
-        else if (dataElement->present == Data_PR_bitstring) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+    }
+    else if (dataElement->present == Data_PR_bitstring) {
+
+        int size = dataElement->choice.bitstring.size;
+
+        if (size > 0) {
 
-            value->type = MMS_BIT_STRING;
-            int size = dataElement->choice.bitstring.size;
+            int maxSize = (size * 8);
+            int bitSize = maxSize - dataElement->choice.bitstring.bits_unused;
 
-            value->value.bitString.size = (size * 8)
-                    - dataElement->choice.bitstring.bits_unused;
+            if ((bitSize > 0) && (maxSize >= bitSize)) {
+                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+
+                value->type = MMS_BIT_STRING;
 
-            value->value.bitString.buf = (uint8_t*) GLOBAL_MALLOC(size);
-            memcpy(value->value.bitString.buf,
-                    dataElement->choice.bitstring.buf, size);
+                value->value.bitString.size = bitSize;
 
+                value->value.bitString.buf = (uint8_t*) GLOBAL_MALLOC(size);
+                memcpy(value->value.bitString.buf,
+                        dataElement->choice.bitstring.buf, size);
+            }
+            else {
+                if (DEBUG_MMS_CLIENT)
+                    printf("MMS CLIENT: error parsing data element (bit string padding problem)!\n");
+            }
         }
-        else if (dataElement->present == Data_PR_floatingpoint) {
-            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-            int size = dataElement->choice.floatingpoint.size;
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing data element (bit string size 0 or negative)!\n");
+        }
+    }
+    else if (dataElement->present == Data_PR_floatingpoint) {
+
+        int size = dataElement->choice.floatingpoint.size;
 
+        if (size == 5) { /* FLOAT32 */
+
+            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
             value->type = MMS_FLOAT;
 
-            if (size == 5) { /* FLOAT32 */
-                value->value.floatingPoint.formatWidth = 32;
-                value->value.floatingPoint.exponentWidth = dataElement->choice.floatingpoint.buf[0];
+            value->value.floatingPoint.formatWidth = 32;
+            value->value.floatingPoint.exponentWidth = dataElement->choice.floatingpoint.buf[0];
 
-                uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1);
+            uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1);
 
-                value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(4);
+            value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(4);
 #if (ORDER_LITTLE_ENDIAN == 1)
-                memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 4);
+            memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 4);
 #else
-                memcpy(value->value.floatingPoint.buf, floatBuf, 4);
+            memcpy(value->value.floatingPoint.buf, floatBuf, 4);
 #endif
-            }
+        }
+
+        if (size == 9) { /* FLOAT64 */
+
+            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+            value->type = MMS_FLOAT;
 
-            if (size == 9) { /* FLOAT64 */
-                value->value.floatingPoint.formatWidth = 64;
-                value->value.floatingPoint.exponentWidth = dataElement->choice.floatingpoint.buf[0];
+            value->value.floatingPoint.formatWidth = 64;
+            value->value.floatingPoint.exponentWidth = dataElement->choice.floatingpoint.buf[0];
 
-                uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1);
+            uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1);
 
-                value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(8);
+            value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(8);
 #if (ORDER_LITTLE_ENDIAN == 1)
-                memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 8);
+            memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 8);
 #else
-                memcpy(value->value.floatingPoint.buf, floatBuf, 8);
+            memcpy(value->value.floatingPoint.buf, floatBuf, 8);
 #endif
-            }
         }
-        else if (dataElement->present == Data_PR_utctime) {
+    }
+    else if (dataElement->present == Data_PR_utctime) {
+
+        int size = dataElement->choice.utctime.size;
+
+        if (size == 8) {
             value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
             value->type = MMS_UTC_TIME;
             memcpy(value->value.utcTime, dataElement->choice.utctime.buf, 8);
         }
-        else if (dataElement->present == Data_PR_octetstring) {
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing UTC time (size is %i instead of 8\n", size);
+        }
+    }
+    else if (dataElement->present == Data_PR_octetstring) {
+
+        if (dataElement->choice.octetstring.size >= 0) {
             value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
             value->type = MMS_OCTET_STRING;
             int size = dataElement->choice.octetstring.size;
@@ -300,20 +379,33 @@ mmsMsg_parseDataElement(Data_t* dataElement)
             value->value.octetString.buf = (uint8_t*) GLOBAL_MALLOC(size);
             memcpy(value->value.octetString.buf, dataElement->choice.octetstring.buf, size);
         }
-        else if (dataElement->present == Data_PR_binarytime) {
-            int size = dataElement->choice.binarytime.size;
 
-            if (size <= 6) {
-                value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
-                value->type = MMS_BINARY_TIME;
-                value->value.binaryTime.size = size;
-                memcpy(value->value.binaryTime.buf, dataElement->choice.binarytime.buf, size);
-            }
+    }
+    else if (dataElement->present == Data_PR_binarytime) {
+        int size = dataElement->choice.binarytime.size;
+
+        if ((size == 4) || (size == 6)) {
+            value = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
+            value->type = MMS_BINARY_TIME;
+            value->value.binaryTime.size = size;
+            memcpy(value->value.binaryTime.buf, dataElement->choice.binarytime.buf, size);
         }
-        else if (dataElement->present == Data_PR_boolean) {
-            value = MmsValue_newBoolean(dataElement->choice.boolean);
+        else {
+            if (DEBUG_MMS_CLIENT)
+                printf("MMS CLIENT: error parsing binary time (size must be 4 or 6, is %i\n", size);
         }
+    }
+    else if (dataElement->present == Data_PR_boolean) {
+        value = MmsValue_newBoolean(dataElement->choice.boolean);
+    }
+    else if (dataElement->present == Data_PR_booleanArray) {
+        if (DEBUG_MMS_CLIENT)
+            printf("MMS CLIENT: unsupported type - boolean-array\n");
+    }
 
+    if (DEBUG_MMS_CLIENT) {
+        if (value == NULL)
+            printf("MMS CLIENT: error parsing data element\n");
     }
 
     return value;

src/mms/iso_mms/common/mms_value.c

@@ -1,7 +1,7 @@
 /*
  *  mms_value.c
  *
- *  Copyright 2013-2018 Michael Zillgith
+ *  Copyright 2013-2019 Michael Zillgith
  *
  *  This file is part of libIEC61850.
  *
@@ -1076,6 +1076,9 @@ MmsValue_cloneToBuffer(const MmsValue* self, uint8_t* destinationAddress)
 MmsValue*
 MmsValue_clone(const MmsValue* self)
 {
+    if (self == NULL)
+        return NULL;
+
     MmsValue* newValue = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue));
 
     if (newValue == NULL)
@@ -1179,6 +1182,9 @@ MmsValue_deleteIfNotNull(MmsValue* self)
 void
 MmsValue_delete(MmsValue* self)
 {
+    if (self == NULL)
+        return;
+
     switch (self->type)
     {
     case MMS_INTEGER:
@@ -1997,6 +2003,16 @@ MmsValue_getTypeString(MmsValue* self)
 const char*
 MmsValue_printToBuffer(const MmsValue* self, char* buffer, int bufferSize)
 {
+    if (self == NULL) {
+        strncpy(buffer, "(null)", bufferSize);
+
+        /* Ensure buffer is always 0 terminated */
+        if (bufferSize > 0)
+            buffer[bufferSize - 1] = 0;
+
+        return buffer;
+    }
+
     switch (MmsValue_getType(self))
     {
     case MMS_STRUCTURE: