From e1eb09d55bd8aab379ccb23633a3de26dd78f9ea Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Sat, 16 May 2020 15:21:40 +0200 Subject: [PATCH] - MmsValue: added NULL checks for all memory allocations - MmsValue: buffer for float/double data is now part of the MmsValue structure and not allocated separately --- src/iec61850/client/ied_connection.c | 2 +- src/mms/asn1/asn1_ber_primitive_value.c | 39 +++- src/mms/asn1/ber_integer.c | 23 +- src/mms/inc_private/mms_value_internal.h | 2 +- src/mms/iso_mms/client/mms_client_read.c | 4 - src/mms/iso_mms/common/mms_common_msg.c | 2 - src/mms/iso_mms/common/mms_value.c | 278 ++++++++++++++--------- 7 files changed, 220 insertions(+), 130 deletions(-) diff --git a/src/iec61850/client/ied_connection.c b/src/iec61850/client/ied_connection.c index 846570af..817d60b1 100644 --- a/src/iec61850/client/ied_connection.c +++ b/src/iec61850/client/ied_connection.c @@ -1633,7 +1633,7 @@ IedConnection_writeFloatValue(IedConnection self, IedClientError* error, const c mmsValue.type = MMS_FLOAT; mmsValue.value.floatingPoint.exponentWidth = 8; mmsValue.value.floatingPoint.formatWidth = 32; - mmsValue.value.floatingPoint.buf = (uint8_t*) &value; + memcpy(mmsValue.value.floatingPoint.buf, (uint8_t*) &value, sizeof(value)); IedConnection_writeObject(self, error, objectReference, fc, &mmsValue); } diff --git a/src/mms/asn1/asn1_ber_primitive_value.c b/src/mms/asn1/asn1_ber_primitive_value.c index 26564ee8..0dbd15bf 100644 --- a/src/mms/asn1/asn1_ber_primitive_value.c +++ b/src/mms/asn1/asn1_ber_primitive_value.c @@ -1,7 +1,7 @@ /* * asn1_ber_primitive_value.c * - * Copyright 2013 Michael Zillgith + * Copyright 2013-2020 Michael Zillgith * * This file is part of libIEC61850. * @@ -30,9 +30,17 @@ Asn1PrimitiveValue_create(int size) { Asn1PrimitiveValue* self = (Asn1PrimitiveValue*) GLOBAL_MALLOC(sizeof(Asn1PrimitiveValue)); - self->size = 1; - self->maxSize = size; - self->octets = (uint8_t*) GLOBAL_CALLOC(1, size); + if (self) { + self->size = 1; + self->maxSize = size; + + self->octets = (uint8_t*) GLOBAL_CALLOC(1, size); + + if (self->octets == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -42,12 +50,21 @@ Asn1PrimitiveValue_clone(Asn1PrimitiveValue* self) { Asn1PrimitiveValue* clone = (Asn1PrimitiveValue*) GLOBAL_MALLOC(sizeof(Asn1PrimitiveValue)); - clone->size = self->size; - clone->maxSize = self->maxSize; + if (clone) { + clone->size = self->size; + clone->maxSize = self->maxSize; - clone->octets = (uint8_t*) GLOBAL_MALLOC(self->maxSize); + clone->octets = (uint8_t*) GLOBAL_MALLOC(self->maxSize); - memcpy(clone->octets, self->octets, clone->maxSize); + if (clone->octets) { + memcpy(clone->octets, self->octets, clone->maxSize); + } + else { + GLOBAL_FREEMEM(clone); + clone = NULL; + } + + } return clone; } @@ -80,6 +97,8 @@ Asn1PrimitiveValue_getMaxSize(Asn1PrimitiveValue* self) void Asn1PrimitiveValue_destroy(Asn1PrimitiveValue* self) { - GLOBAL_FREEMEM(self->octets); - GLOBAL_FREEMEM(self); + if (self) { + GLOBAL_FREEMEM(self->octets); + GLOBAL_FREEMEM(self); + } } diff --git a/src/mms/asn1/ber_integer.c b/src/mms/asn1/ber_integer.c index 36789971..39bac880 100644 --- a/src/mms/asn1/ber_integer.c +++ b/src/mms/asn1/ber_integer.c @@ -1,7 +1,7 @@ /* * ber_integer.c * - * Copyright 2013-2019 Michael Zillgith + * Copyright 2013-2020 Michael Zillgith * * This file is part of libIEC61850. * @@ -61,9 +61,11 @@ BerInteger_createFromBuffer(uint8_t* buf, int size) Asn1PrimitiveValue* self = Asn1PrimitiveValue_create(maxSize); - memcpy(self->octets, buf, size); + if (self) { + memcpy(self->octets, buf, size); - self->size = size; + self->size = size; + } return self; } @@ -95,7 +97,10 @@ Asn1PrimitiveValue* BerInteger_createFromInt32(int32_t value) { Asn1PrimitiveValue* asn1Value = BerInteger_createInt32(); - BerInteger_setInt32(asn1Value, value); + + if (asn1Value) { + BerInteger_setInt32(asn1Value, value); + } return asn1Value; } @@ -163,7 +168,10 @@ Asn1PrimitiveValue* BerInteger_createFromUint32(uint32_t value) { Asn1PrimitiveValue* asn1Value = BerInteger_createInt32(); - BerInteger_setUint32(asn1Value, value); + + if (asn1Value) { + BerInteger_setUint32(asn1Value, value); + } return asn1Value; } @@ -187,7 +195,10 @@ Asn1PrimitiveValue* BerInteger_createFromInt64(int64_t value) { Asn1PrimitiveValue* asn1Value = BerInteger_createInt64(); - BerInteger_setInt64(asn1Value, value); + + if (asn1Value) { + BerInteger_setInt64(asn1Value, value); + } return asn1Value; } diff --git a/src/mms/inc_private/mms_value_internal.h b/src/mms/inc_private/mms_value_internal.h index 20b89209..ebbc1672 100644 --- a/src/mms/inc_private/mms_value_internal.h +++ b/src/mms/inc_private/mms_value_internal.h @@ -41,7 +41,7 @@ struct ATTRIBUTE_PACKED sMmsValue { struct { uint8_t exponentWidth; uint8_t formatWidth; /* number of bits - either 32 or 64) */ - uint8_t* buf; + uint8_t buf[8]; } floatingPoint; struct { uint16_t size; diff --git a/src/mms/iso_mms/client/mms_client_read.c b/src/mms/iso_mms/client/mms_client_read.c index 795cff9c..ae9add45 100644 --- a/src/mms/iso_mms/client/mms_client_read.c +++ b/src/mms/iso_mms/client/mms_client_read.c @@ -206,8 +206,6 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi uint8_t* floatBuf = (accessResultList[i]->choice.floatingpoint.buf + 1); - value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(4); - #if (ORDER_LITTLE_ENDIAN == 1) memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 4); #else @@ -225,8 +223,6 @@ mmsClient_parseListOfAccessResults(AccessResult_t** accessResultList, int listSi uint8_t* floatBuf = (accessResultList[i]->choice.floatingpoint.buf + 1); - value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(8); - #if (ORDER_LITTLE_ENDIAN == 1) memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 8); #else diff --git a/src/mms/iso_mms/common/mms_common_msg.c b/src/mms/iso_mms/common/mms_common_msg.c index 857119a0..7f6e7501 100644 --- a/src/mms/iso_mms/common/mms_common_msg.c +++ b/src/mms/iso_mms/common/mms_common_msg.c @@ -336,7 +336,6 @@ mmsMsg_parseDataElement(Data_t* dataElement) uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1); - value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(4); #if (ORDER_LITTLE_ENDIAN == 1) memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 4); #else @@ -354,7 +353,6 @@ mmsMsg_parseDataElement(Data_t* dataElement) uint8_t* floatBuf = (dataElement->choice.floatingpoint.buf + 1); - value->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(8); #if (ORDER_LITTLE_ENDIAN == 1) memcpyReverseByteOrder(value->value.floatingPoint.buf, floatBuf, 8); #else diff --git a/src/mms/iso_mms/common/mms_value.c b/src/mms/iso_mms/common/mms_value.c index 106ea7b0..8b9fc5d0 100644 --- a/src/mms/iso_mms/common/mms_value.c +++ b/src/mms/iso_mms/common/mms_value.c @@ -1,7 +1,7 @@ /* * mms_value.c * - * Copyright 2013-2019 Michael Zillgith + * Copyright 2013-2020 Michael Zillgith * * This file is part of libIEC61850. * @@ -275,6 +275,10 @@ MmsValue_update(MmsValue* self, const MmsValue* update) if (size > self->value.octetString.maxSize) { GLOBAL_FREEMEM(self->value.octetString.buf); self->value.octetString.buf = (uint8_t*) GLOBAL_MALLOC(size); + + if (self->value.octetString.buf == NULL) + return false; + self->value.octetString.maxSize = size; } @@ -330,6 +334,11 @@ MmsValue_newBitString(int bitSize) self->value.bitString.size = abs(bitSize); self->value.bitString.buf = (uint8_t*) GLOBAL_CALLOC(bitStringByteSize(self), 1); + if (self->value.bitString.buf == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + return self; } @@ -531,15 +540,13 @@ MmsValue_newFloat(float variable) { MmsValue* self = (MmsValue*) GLOBAL_MALLOC(sizeof(MmsValue)); - if (self == NULL) - return NULL; - - self->type = MMS_FLOAT; - self->value.floatingPoint.formatWidth = 32; - self->value.floatingPoint.exponentWidth = 8; - self->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(4); + if (self) { + self->type = MMS_FLOAT; + self->value.floatingPoint.formatWidth = 32; + self->value.floatingPoint.exponentWidth = 8; - *((float*) self->value.floatingPoint.buf) = variable; + *((float*) self->value.floatingPoint.buf) = variable; + } return self; } @@ -575,15 +582,13 @@ MmsValue_newDouble(double variable) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; - - self->type = MMS_FLOAT; - self->value.floatingPoint.formatWidth = 64; - self->value.floatingPoint.exponentWidth = 11; - self->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(8); + if (self) { + self->type = MMS_FLOAT; + self->value.floatingPoint.formatWidth = 64; + self->value.floatingPoint.exponentWidth = 11; - *((double*) self->value.floatingPoint.buf) = variable; + *((double*) self->value.floatingPoint.buf) = variable; + } return self; } @@ -593,8 +598,15 @@ MmsValue_newIntegerFromInt8(int8_t integer) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - self->type = MMS_INTEGER; - self->value.integer = BerInteger_createFromInt32((int32_t) integer); + if (self) { + self->type = MMS_INTEGER; + self->value.integer = BerInteger_createFromInt32((int32_t) integer); + + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -604,8 +616,15 @@ MmsValue_newIntegerFromInt16(int16_t integer) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - self->type = MMS_INTEGER; - self->value.integer = BerInteger_createFromInt32((int32_t) integer); + if (self) { + self->type = MMS_INTEGER; + self->value.integer = BerInteger_createFromInt32((int32_t) integer); + + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -825,11 +844,15 @@ MmsValue_newIntegerFromInt32(int32_t integer) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_INTEGER; + self->value.integer = BerInteger_createFromInt32(integer); - self->type = MMS_INTEGER; - self->value.integer = BerInteger_createFromInt32(integer); + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -839,11 +862,15 @@ MmsValue_newUnsignedFromUint32(uint32_t integer) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_UNSIGNED; + self->value.integer = BerInteger_createFromUint32(integer); - self->type = MMS_UNSIGNED; - self->value.integer = BerInteger_createFromUint32(integer); + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -853,11 +880,15 @@ MmsValue_newIntegerFromInt64(int64_t integer) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_INTEGER; + self->value.integer = BerInteger_createFromInt64(integer); - self->type = MMS_INTEGER; - self->value.integer = BerInteger_createFromInt64(integer); + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -988,10 +1019,6 @@ MmsValue_getSizeInMemory(const MmsValue* self) memorySize += MemoryAllocator_getAlignedSize(self->value.integer->maxSize); break; - case MMS_FLOAT: - memorySize += MemoryAllocator_getAlignedSize(self->value.floatingPoint.formatWidth / 8); - break; - case MMS_OCTET_STRING: memorySize += MemoryAllocator_getAlignedSize(self->value.octetString.maxSize); break; @@ -1051,16 +1078,6 @@ MmsValue_cloneToBuffer(const MmsValue* self, uint8_t* destinationAddress) } break; - case MMS_FLOAT: - { - int floatSizeInBytes = (self->value.floatingPoint.formatWidth / 8); - - newValue->value.floatingPoint.buf = destinationAddress; - memcpy(destinationAddress, self->value.floatingPoint.buf, floatSizeInBytes); - destinationAddress += MemoryAllocator_getAlignedSize(floatSizeInBytes); - } - break; - case MMS_OCTET_STRING: newValue->value.octetString.buf = destinationAddress; memcpy(destinationAddress, self->value.octetString.buf, self->value.octetString.maxSize); @@ -1106,24 +1123,51 @@ MmsValue_clone(const MmsValue* self) int componentCount = self->value.structure.size; newValue->value.structure.size = componentCount; newValue->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*)); - int i; - for (i = 0; i < componentCount; i++) { - newValue->value.structure.components[i] = - MmsValue_clone(self->value.structure.components[i]); + + if (newValue->value.structure.components) { + int i; + for (i = 0; i < componentCount; i++) { + + if (self->value.structure.components[i]) { + newValue->value.structure.components[i] = + MmsValue_clone(self->value.structure.components[i]); + + if (newValue->value.structure.components[i] == NULL) { + MmsValue_delete(newValue); + newValue = NULL; + break; + } + } + else { + newValue->value.structure.components[i] = NULL; + } + + } } + else { + GLOBAL_FREEMEM(newValue); + newValue = NULL; + } + } break; case MMS_INTEGER: case MMS_UNSIGNED: - newValue->value.integer = Asn1PrimitiveValue_clone(self->value.integer); + { + newValue->value.integer = Asn1PrimitiveValue_clone(self->value.integer); + + if (newValue->value.integer == NULL) { + GLOBAL_FREEMEM(newValue); + newValue = NULL; + } + } break; case MMS_FLOAT: newValue->value.floatingPoint.formatWidth = self->value.floatingPoint.formatWidth; newValue->value.floatingPoint.exponentWidth = self->value.floatingPoint.exponentWidth; size = self->value.floatingPoint.formatWidth / 8; - newValue->value.floatingPoint.buf = (uint8_t*) GLOBAL_MALLOC(size); memcpy(newValue->value.floatingPoint.buf, self->value.floatingPoint.buf, size); break; @@ -1193,9 +1237,6 @@ MmsValue_delete(MmsValue* self) case MMS_UNSIGNED: Asn1PrimitiveValue_destroy(self->value.integer); break; - case MMS_FLOAT: - GLOBAL_FREEMEM(self->value.floatingPoint.buf); - break; case MMS_BIT_STRING: if (self->value.bitString.buf != NULL) GLOBAL_FREEMEM(self->value.bitString.buf); @@ -1240,9 +1281,6 @@ MmsValue_deleteConditional(MmsValue* self) case MMS_UNSIGNED: Asn1PrimitiveValue_destroy(self->value.integer); break; - case MMS_FLOAT: - GLOBAL_FREEMEM(self->value.floatingPoint.buf); - break; case MMS_BIT_STRING: GLOBAL_FREEMEM(self->value.bitString.buf); break; @@ -1280,15 +1318,19 @@ MmsValue_newInteger(int size /*integer size in bits*/) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_INTEGER; - self->type = MMS_INTEGER; + if (size <= 32) + self->value.integer = BerInteger_createInt32(); + else + self->value.integer = BerInteger_createInt64(); - if (size <= 32) - self->value.integer = BerInteger_createInt32(); - else - self->value.integer = BerInteger_createInt64(); + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -1298,15 +1340,19 @@ MmsValue_newUnsigned(int size /*integer size in bits*/) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_UNSIGNED; - self->type = MMS_UNSIGNED; + if (size <= 32) + self->value.integer = BerInteger_createInt32(); + else + self->value.integer = BerInteger_createInt64(); - if (size <= 32) - self->value.integer = BerInteger_createInt32(); - else - self->value.integer = BerInteger_createInt64(); + if (self->value.integer == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -1316,15 +1362,14 @@ MmsValue_newBoolean(bool boolean) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; - - self->type = MMS_BOOLEAN; + if (self) { + self->type = MMS_BOOLEAN; - if (boolean == true) - self->value.boolean = 1; - else - self->value.boolean = 0; + if (boolean) + self->value.boolean = 1; + else + self->value.boolean = 0; + } return self; } @@ -1334,13 +1379,17 @@ MmsValue_newOctetString(int size, int maxSize) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_OCTET_STRING; + self->value.octetString.size = size; + self->value.octetString.maxSize = maxSize; + self->value.octetString.buf = (uint8_t*) GLOBAL_CALLOC(1, maxSize); - self->type = MMS_OCTET_STRING; - self->value.octetString.size = size; - self->value.octetString.maxSize = maxSize; - self->value.octetString.buf = (uint8_t*) GLOBAL_CALLOC(1, maxSize); + if (self->value.octetString.buf == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + } + } return self; } @@ -1377,18 +1426,29 @@ MmsValue_newStructure(const MmsVariableSpecification* typeSpec) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; + if (self) { + self->type = MMS_STRUCTURE; + int componentCount = typeSpec->typeSpec.structure.elementCount; + self->value.structure.size = componentCount; + self->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*)); - self->type = MMS_STRUCTURE; - int componentCount = typeSpec->typeSpec.structure.elementCount; - self->value.structure.size = componentCount; - self->value.structure.components = (MmsValue**) GLOBAL_CALLOC(componentCount, sizeof(MmsValue*)); + if (self->value.structure.components) { + int i; + for (i = 0; i < componentCount; i++) { + self->value.structure.components[i] = + MmsValue_newDefaultValue(typeSpec->typeSpec.structure.elements[i]); - int i; - for (i = 0; i < componentCount; i++) { - self->value.structure.components[i] = - MmsValue_newDefaultValue(typeSpec->typeSpec.structure.elements[i]); + if (self->value.structure.components[i] == NULL) { + MmsValue_delete(self); + self = NULL; + break; + } + } + } + else { + GLOBAL_FREEMEM(self); + self = NULL; + } } return self; @@ -1418,7 +1478,7 @@ MmsValue_newDefaultValue(const MmsVariableSpecification* typeSpec) self->type = MMS_FLOAT; self->value.floatingPoint.exponentWidth = typeSpec->typeSpec.floatingpoint.exponentWidth; self->value.floatingPoint.formatWidth = typeSpec->typeSpec.floatingpoint.formatWidth; - self->value.floatingPoint.buf = (uint8_t*) GLOBAL_CALLOC(1, typeSpec->typeSpec.floatingpoint.formatWidth / 8); + memset(self->value.floatingPoint.buf, 0, 8); break; case MMS_BIT_STRING: @@ -1451,6 +1511,13 @@ MmsValue_newDefaultValue(const MmsVariableSpecification* typeSpec) self->value.octetString.maxSize = abs(typeSpec->typeSpec.octetString); self->value.octetString.buf = (uint8_t*) GLOBAL_CALLOC(1, abs(typeSpec->typeSpec.octetString)); + + if (self->value.octetString.buf == NULL) { + GLOBAL_FREEMEM(self); + self = NULL; + goto exit_function; + } + break; case MMS_VISIBLE_STRING: @@ -1617,15 +1684,14 @@ MmsValue_newBinaryTime(bool timeOfDay) { MmsValue* self = (MmsValue*) GLOBAL_CALLOC(1, sizeof(MmsValue)); - if (self == NULL) - return NULL; - - self->type = MMS_BINARY_TIME; + if (self) { + self->type = MMS_BINARY_TIME; - if (timeOfDay == true) - self->value.binaryTime.size = 4; - else - self->value.binaryTime.size = 6; + if (timeOfDay == true) + self->value.binaryTime.size = 4; + else + self->value.binaryTime.size = 6; + } return self; }