From f41667367a0c9841400bd3e50415db8a5301b7ad Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Tue, 2 May 2023 19:17:25 +0100 Subject: [PATCH] - GOOSE subscriber: fixed - possible heap corruption in parseAllData due to missing validity check in bit-string handling (LIB61850-402) --- hal/ethernet/linux/ethernet_linux.c | 2 ++ src/common/string_utilities.c | 3 +++ src/goose/goose_receiver.c | 42 ++++++++++++++++++++++------- src/goose/goose_subscriber.h | 1 + src/mms/iso_mms/common/mms_value.c | 10 ++++--- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/hal/ethernet/linux/ethernet_linux.c b/hal/ethernet/linux/ethernet_linux.c index 2d97a826..eaf1897a 100644 --- a/hal/ethernet/linux/ethernet_linux.c +++ b/hal/ethernet/linux/ethernet_linux.c @@ -263,6 +263,8 @@ void Ethernet_addMulticastAddress(EthernetSocket ethSocket, uint8_t* multicastAddress) { struct packet_mreq mreq; + memset(&mreq, 0, sizeof(struct packet_mreq)); + mreq.mr_ifindex = ethSocket->socketAddress.sll_ifindex; mreq.mr_alen = ETH_ALEN; mreq.mr_type = PACKET_MR_MULTICAST; diff --git a/src/common/string_utilities.c b/src/common/string_utilities.c index dfb9d620..37e62ad7 100644 --- a/src/common/string_utilities.c +++ b/src/common/string_utilities.c @@ -194,6 +194,9 @@ StringUtils_copyStringMax(char* dest, int maxBufferSize, const char* str1) { char* res = dest; + if (maxBufferSize < 1) + return NULL; + if (dest == NULL) res = (char*)GLOBAL_MALLOC(maxBufferSize); diff --git a/src/goose/goose_receiver.c b/src/goose/goose_receiver.c index 06160008..63fe9710 100644 --- a/src/goose/goose_receiver.c +++ b/src/goose/goose_receiver.c @@ -210,13 +210,22 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues) case 0x84: /* BIT STRING */ if (MmsValue_getType(value) == MMS_BIT_STRING) { int padding = buffer[bufPos]; - int bitStringLength = (8 * (elementLength - 1)) - padding; - if (bitStringLength == value->value.bitString.size) { - memcpy(value->value.bitString.buf, buffer + bufPos + 1, - elementLength - 1); + + if (padding > 7) { + if (DEBUG_GOOSE_SUBSCRIBER) + printf("GOOSE_SUBSCRIBER: invalid bit-string (padding not plausible)\n"); + + pe = GOOSE_PARSE_ERROR_INVALID_PADDING; } else { - pe = GOOSE_PARSE_ERROR_LENGTH_MISMATCH; + int bitStringLength = (8 * (elementLength - 1)) - padding; + if (bitStringLength == value->value.bitString.size) { + memcpy(value->value.bitString.buf, buffer + bufPos + 1, + elementLength - 1); + } + else { + pe = GOOSE_PARSE_ERROR_LENGTH_MISMATCH; + } } } else { @@ -352,7 +361,7 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues) break; } - if ( pe != GOOSE_PARSE_ERROR_NO_ERROR ) { + if (pe != GOOSE_PARSE_ERROR_NO_ERROR) { break; /* from while */ } @@ -362,14 +371,16 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues) } if (elementIndex <= maxIndex) { - pe = GOOSE_PARSE_ERROR_UNDERFLOW; + if (pe == GOOSE_PARSE_ERROR_NO_ERROR) { + pe = GOOSE_PARSE_ERROR_UNDERFLOW; + } } if (DEBUG_GOOSE_SUBSCRIBER) { - switch ( pe ) { + switch (pe) { case GOOSE_PARSE_ERROR_UNKNOWN_TAG: printf("GOOSE_SUBSCRIBER: Found unkown tag %02x!\n", tag); - break; + break; case GOOSE_PARSE_ERROR_TAGDECODE: printf("GOOSE_SUBSCRIBER: Malformed message: failed to decode BER length tag!\n"); break; @@ -388,6 +399,8 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues) case GOOSE_PARSE_ERROR_LENGTH_MISMATCH: printf("GOOSE_SUBSCRIBER: Message contains value of wrong length!\n"); break; + case GOOSE_PARSE_ERROR_INVALID_PADDING: + printf("GOOSE_SUBSCRIBER: Malformed message: invalid padding!\n"); default: break; } @@ -500,7 +513,16 @@ parseAllDataUnknownValue(GooseSubscriber self, uint8_t* buffer, int allDataLengt case 0x83: /* boolean */ if (DEBUG_GOOSE_SUBSCRIBER) printf("GOOSE_SUBSCRIBER: found boolean\n"); - value = MmsValue_newBoolean(BerDecoder_decodeBoolean(buffer, bufPos)); + + if (elementLength > 0) { + value = MmsValue_newBoolean(BerDecoder_decodeBoolean(buffer, bufPos)); + } + else { + if (DEBUG_GOOSE_SUBSCRIBER) + printf("GOOSE_SUBSCRIBER: invalid length for boolean\n"); + + goto exit_with_error; + } break; diff --git a/src/goose/goose_subscriber.h b/src/goose/goose_subscriber.h index e58f4f96..7408d6b5 100644 --- a/src/goose/goose_subscriber.h +++ b/src/goose/goose_subscriber.h @@ -47,6 +47,7 @@ typedef enum GOOSE_PARSE_ERROR_UNDERFLOW, GOOSE_PARSE_ERROR_TYPE_MISMATCH, GOOSE_PARSE_ERROR_LENGTH_MISMATCH, + GOOSE_PARSE_ERROR_INVALID_PADDING } GooseParseError; typedef struct sGooseSubscriber* GooseSubscriber; diff --git a/src/mms/iso_mms/common/mms_value.c b/src/mms/iso_mms/common/mms_value.c index 29e96e4d..549619a8 100644 --- a/src/mms/iso_mms/common/mms_value.c +++ b/src/mms/iso_mms/common/mms_value.c @@ -2205,7 +2205,7 @@ MmsValue_printToBuffer(const MmsValue* self, char* buffer, int bufferSize) const char* currentStr = MmsValue_printToBuffer((const MmsValue*) MmsValue_getElement(self, i), buffer + bufPos, bufferSize - bufPos); - bufPos += strlen(currentStr); + bufPos += strnlen(currentStr, bufferSize - bufPos); if (bufPos >= bufferSize) break; @@ -2240,9 +2240,13 @@ MmsValue_printToBuffer(const MmsValue* self, char* buffer, int bufferSize) int size = MmsValue_getBitStringSize(self); /* fill buffer with zeros */ - if (size > bufferSize) { + if (size + 1 > bufferSize) { memset(buffer, 0, bufferSize); - break; + + size = bufferSize - 1; + + if (size < 1) + break; } int i;