- 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

@@ -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;

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);
 

src/goose/goose_receiver.c

@@ -210,6 +210,14 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues)
         case 0x84: /* BIT STRING */
             if (MmsValue_getType(value) == MMS_BIT_STRING) {
                 int padding = buffer[bufPos];
+
+                if (padding > 7) {
+                    if (DEBUG_GOOSE_SUBSCRIBER)
+                        printf("GOOSE_SUBSCRIBER:      invalid bit-string (padding not plausible)\n");
+
+                    pe = GOOSE_PARSE_ERROR_INVALID_PADDING;
+                }
+                else {
                     int bitStringLength = (8 * (elementLength - 1)) - padding;
                     if (bitStringLength == value->value.bitString.size) {
                         memcpy(value->value.bitString.buf, buffer + bufPos + 1,
@@ -219,6 +227,7 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues)
                         pe = GOOSE_PARSE_ERROR_LENGTH_MISMATCH;
                     }
                 }
+            }
             else {
                 pe = GOOSE_PARSE_ERROR_TYPE_MISMATCH;
             }
@@ -362,8 +371,10 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues)
     }
 
     if (elementIndex <= maxIndex) {
+        if (pe == GOOSE_PARSE_ERROR_NO_ERROR) {
             pe = GOOSE_PARSE_ERROR_UNDERFLOW;
         }
+    }
 
     if (DEBUG_GOOSE_SUBSCRIBER) {
         switch (pe) {
@@ -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");
+
+            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;
 

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;

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,8 +2240,12 @@ 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);
+
+                size = bufferSize - 1;
+
+                if (size < 1)
                     break;
             }