- added additional length check in BerDecoder_decodeLength and checked return values in callers (fix problem #196)

src/goose/goose_receiver.c

@@ -157,12 +157,6 @@ parseAllData(uint8_t* buffer, int allDataLength, MmsValue* dataSetValues)
             return 0;
         }
 
-        if (bufPos + elementLength > allDataLength) {
-            if (DEBUG_GOOSE_SUBSCRIBER)
-                printf("GOOSE_SUBSCRIBER: Malformed message: sub element is too large!\n");
-            return 0;
-        }
-
         switch (tag)
         {
         case 0x80: /* reserved for access result */
@@ -328,12 +322,6 @@ parseAllDataUnknownValue(GooseSubscriber self, uint8_t* buffer, int allDataLengt
             return 0;
         }
 
-        if (bufPos + elementLength > allDataLength) {
-            if (DEBUG_GOOSE_SUBSCRIBER)
-                printf("GOOSE_SUBSCRIBER: Malformed message: sub element is too large!\n");
-            goto exit_with_error;
-        }
-
         switch (tag)
         {
         case 0x80: /* reserved for access result */
@@ -389,12 +377,6 @@ parseAllDataUnknownValue(GooseSubscriber self, uint8_t* buffer, int allDataLengt
             return 0;
         }
 
-        if (bufPos + elementLength > allDataLength) {
-            if (DEBUG_GOOSE_SUBSCRIBER)
-                printf("GOOSE_SUBSCRIBER: Malformed message: sub element is too large!\n");
-            goto exit_with_error;
-        }
-
         MmsValue* value = NULL;
 
         switch (tag)
@@ -552,13 +534,6 @@ parseGoosePayload(GooseReceiver self, uint8_t* buffer, int apduLength)
                 return 0;
             }
 
-            if (bufPos + elementLength > apduLength) {
-                if (DEBUG_GOOSE_SUBSCRIBER)
-                    printf("GOOSE_SUBSCRIBER: Malformed message: sub element is too large!\n");
-
-                goto exit_with_fault;
-            }
-
             if (bufPos == -1)
                 goto exit_with_fault;
 

src/iec61850/server/mms_mapping/reporting.c

@@ -2823,6 +2823,13 @@ sendNextReportEntrySegment(ReportControl* self)
 
                 int lenSize = BerDecoder_decodeLength(currentReportBufferPos + 1, &length, 0, report->entryLength);
 
+                if (lenSize < 0) {
+                    if (DEBUG_IED_SERVER)
+                        printf("IED_SERVER: internal error in report buffer\n");
+
+                    return false;
+                }
+
                 int dataElementSize =  1 + lenSize + length;
 
                 elementSize += dataElementSize;
@@ -2999,6 +3006,13 @@ sendNextReportEntrySegment(ReportControl* self)
 
             int lenSize = BerDecoder_decodeLength(currentReportBufferPos + 1, &length, 0, report->entryLength);
 
+            if (lenSize < 0) {
+                if (DEBUG_IED_SERVER)
+                    printf("IED_SERVER: internal error in report buffer\n");
+
+                return false;
+            }
+
             int dataElementSize =  1 + lenSize + length;
 
             if (i >= startElementIndex) {

src/mms/asn1/ber_decode.c

@@ -59,6 +59,9 @@ BerDecoder_decodeLength(uint8_t* buffer, int* length, int bufPos, int maxBufPos)
     if (*length < 0)
         return -1;
 
+    if (*length > maxBufPos)
+        return -1;
+
     if (bufPos + (*length) > maxBufPos)
         return -1;
 

src/mms/iso_mms/client/mms_client_connection.c

@@ -580,9 +580,6 @@ mmsMsg_parseConfirmedErrorPDU(uint8_t* buffer, int bufPos, int maxBufPos, uint32
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos)
-        goto exit_error;
-
     while (bufPos < endPos) {
         tag = buffer[bufPos++];
 
@@ -640,14 +637,8 @@ mmsMsg_parseRejectPDU(uint8_t* buffer, int bufPos, int maxBufPos, uint32_t* invo
     if (bufPos < 0)
         goto exit_error;
 
-    if (bufPos + length > maxBufPos)
-        goto exit_error;
-
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos)
-        goto exit_error;
-
     while (bufPos < endPos) {
         tag = buffer[bufPos++];
 
@@ -1217,14 +1208,14 @@ mmsIsoCallback(IsoIndication indication, void* parameter, ByteBuffer* payload)
         int bufPos = 1;
 
         bufPos = BerDecoder_decodeLength(buf, &length, bufPos, payload->size);
-        if (bufPos == -1)
+        if (bufPos < 0)
             goto exit_with_error;
 
         if (buf[bufPos++] == 0x02) {
             int invokeIdLength;
 
             bufPos = BerDecoder_decodeLength(buf, &invokeIdLength, bufPos, payload->size);
-            if (bufPos == -1)
+            if (bufPos < 0)
                 goto exit_with_error;
 
             uint32_t invokeId =
@@ -1270,7 +1261,7 @@ mmsIsoCallback(IsoIndication indication, void* parameter, ByteBuffer* payload)
         int length;
 
         bufPos = BerDecoder_decodeLength(buf, &length, bufPos, payload->size);
-        if (bufPos == -1)
+        if (bufPos < 0)
             goto exit_with_error;
 
         bool hasInvokeId = false;
@@ -1288,7 +1279,7 @@ mmsIsoCallback(IsoIndication indication, void* parameter, ByteBuffer* payload)
             }
 
             bufPos = BerDecoder_decodeLength(buf, &length, bufPos, payload->size);
-            if (bufPos == -1)
+            if (bufPos < 0)
                 goto exit_with_error;
 
             if (extendedTag) {

src/mms/iso_mms/client/mms_client_files.c

@@ -99,8 +99,6 @@ mmsClient_handleFileOpenRequest(
 
         if (bufPos < 0) goto exit_reject_invalid_pdu;
 
-        if (bufPos + length > maxBufPos) goto exit_reject_invalid_pdu;
-
         switch(tag) {
         case 0xa0: /* filename */
 
@@ -497,7 +495,7 @@ parseDirectoryEntry(uint8_t* buffer, int bufPos, int maxBufPos, uint32_t invokeI
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
         if (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
-                printf("MMS_CLIENT: message contains unknown tag!\n");
+                printf("MMS_CLIENT: invalid length field\n");
             return false;
         }
 
@@ -511,7 +509,7 @@ parseDirectoryEntry(uint8_t* buffer, int bufPos, int maxBufPos, uint32_t invokeI
             bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
             if (bufPos < 0) {
                 if (DEBUG_MMS_CLIENT)
-                    printf("MMS_CLIENT: message contains unknown tag!\n");
+                    printf("MMS_CLIENT: invalid length field\n");
                 return false;
             }
 
@@ -559,12 +557,6 @@ parseListOfDirectoryEntries(uint8_t* buffer, int bufPos, int maxBufPos, uint32_t
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-         if (DEBUG_MMS_CLIENT)
-             printf("parseListOfDirectoryEntries: message to short!\n");
-         return false;
-    }
-
     while (bufPos < endPos) {
         tag = buffer[bufPos++];
 
@@ -616,12 +608,6 @@ mmsClient_parseFileDirectoryResponse(ByteBuffer* response, int bufPos, uint32_t
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("mmsClient_parseFileDirectoryResponse: message to short (length:%i maxBufPos:%i)!\n", length, maxBufPos);
-        return false;
-    }
-
     bool moreFollows = false;
 
     while (bufPos < endPos) {
@@ -680,12 +666,6 @@ mmsMsg_parseFileOpenResponse(uint8_t* buffer, int bufPos, int maxBufPos, int32_t
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("MMS_CLIENT/SERVER: mmsClient_parseFileOpenResponse: message to short (length:%i maxBufPos:%i)!\n", length, maxBufPos);
-        return false;
-    }
-
     while (bufPos < endPos) {
         tag = buffer[bufPos++];
 
@@ -748,14 +728,9 @@ mmsMsg_parseFileReadResponse(uint8_t* buffer, int bufPos, int maxBufPos, uint32_
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("MMS_CLIENT/SERVER: mmsClient_parseFileReadResponse: message to short (length:%i maxBufPos:%i)!\n", length, maxBufPos);
-        return false;
-    }
-
     while (bufPos < endPos) {
         tag = buffer[bufPos++];
+
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
         if (bufPos < 0)
             return false;

src/mms/iso_mms/client/mms_client_get_namelist.c

@@ -151,7 +151,6 @@ mmsClient_parseGetNameListResponse(LinkedList* nameList, ByteBuffer* message)
     if (bufPos < 0) goto exit_error;
 
     int listEndPos = bufPos + length;
-    if (listEndPos > maxBufPos) goto exit_error;
 
     if (*nameList == NULL)
         *nameList = LinkedList_create();
@@ -174,10 +173,15 @@ mmsClient_parseGetNameListResponse(LinkedList* nameList, ByteBuffer* message)
 
     if (bufPos < maxBufPos) {
 		tag = buffer[bufPos++];
+
 		if (tag != 0x81) goto exit_error;
+
 		bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
+
 		if (bufPos < 0) goto exit_error;
+
 		if (length != 1) goto exit_error;
+
 		if (buffer[bufPos++] > 0)
 			moreFollows = true;
 		else

src/mms/iso_mms/client/mms_client_identify.c

@@ -66,12 +66,6 @@ mmsClient_parseIdentifyResponse(MmsConnection self, ByteBuffer* response, uint32
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("mmsClient_parseIdentifyResponse: Message to short!\n");
-        goto exit_error;
-    }
-
     char vendorNameBuf[100];
     char modelNameBuf[100];
     char revisionBuf[100];

src/mms/iso_mms/client/mms_client_initiate.c

@@ -178,9 +178,6 @@ mmsClient_parseInitiateResponse(MmsConnection self, ByteBuffer* response)
     if (bufPos < 0)
         return false;
 
-    if (bufPos + length > maxBufPos)
-        return false;
-
     while (bufPos < maxBufPos) {
         uint8_t tag = buffer[bufPos++];
 
@@ -189,9 +186,6 @@ mmsClient_parseInitiateResponse(MmsConnection self, ByteBuffer* response)
         if (bufPos < 0)
             return false;
 
-        if (bufPos + length > maxBufPos)
-            return false;
-
         switch (tag) {
         case 0x80: /* local-detail-calling */
             self->parameters.maxPduSize = BerDecoder_decodeUint32(buffer, length, bufPos);

src/mms/iso_mms/client/mms_client_journals.c

@@ -46,7 +46,7 @@ parseJournalVariable(uint8_t* buffer, int bufPos, int maxLength, MmsJournalVaria
 
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if  ((bufPos < 0) || ((bufPos + length) > maxBufPos)) { /* check length field for validity */
+        if  (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
                 printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -94,7 +94,7 @@ parseJournalVariables(uint8_t* buffer, int bufPos, int maxLength, MmsJournalEntr
         uint8_t tag = buffer[bufPos++];
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos < 0) || ((bufPos + length) > maxBufPos)) { /* check length field for validity */
+        if (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
                 printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -117,7 +117,6 @@ parseJournalVariables(uint8_t* buffer, int bufPos, int maxLength, MmsJournalEntr
 
         default:
             break;
-
         }
 
         bufPos += length;
@@ -137,7 +136,7 @@ parseData(uint8_t* buffer, int bufPos, int maxLength, MmsJournalEntry journalEnt
         uint8_t tag = buffer[bufPos++];
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos < 0) || ((bufPos + length) > maxBufPos)) { /* check length field for validity */
+        if (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
                 printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -175,7 +174,7 @@ parseEntryContent(uint8_t* buffer, int bufPos, int maxLength, MmsJournalEntry jo
        uint8_t tag = buffer[bufPos++];
        bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-       if ((bufPos < 0) ||((bufPos + length) > maxBufPos)) { /* check length field for validity */
+       if (bufPos < 0) {
            if (DEBUG_MMS_CLIENT)
                printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -227,7 +226,7 @@ parseJournalEntry(uint8_t* buffer, int bufPos, int maxLength, LinkedList journal
         uint8_t tag = buffer[bufPos++];
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos + length) > maxBufPos) { /* check length field for validity */
+        if (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
                 printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -276,7 +275,7 @@ parseListOfJournalEntries(uint8_t* buffer, int bufPos, int maxLength, LinkedList
         uint8_t tag = buffer[bufPos++];
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos < 0) || ((bufPos + length) > maxBufPos)) { /* check length field for validity */
+        if (bufPos < 0) {
             if (DEBUG_MMS_CLIENT)
                 printf("MMS_CLIENT: parseReadJournalResponse: invalid length field\n");
 
@@ -334,12 +333,6 @@ mmsClient_parseReadJournalResponse(MmsConnection self, ByteBuffer* response, int
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("MMS_CLIENT: mmsClient_parseReadJournalResponse: message to short (length:%i maxBufPos:%i)!\n", length, maxBufPos);
-        return false;
-    }
-
     LinkedList journalEntries = NULL;
 
     while (bufPos < endPos) {

src/mms/iso_mms/client/mms_client_status.c

@@ -69,12 +69,6 @@ mmsClient_parseStatusResponse(MmsConnection self, ByteBuffer* response, int bufP
 
     int endPos = bufPos + length;
 
-    if (endPos > maxBufPos) {
-        if (DEBUG_MMS_CLIENT)
-            printf("mmsClient_parseStatusResponse: message to short!\n");
-        goto exit_error;
-    }
-
     bool hasPhysicalStatus = false;
     bool hasLogicalStatus = false;
 

src/mms/iso_mms/client/mms_client_write.c

@@ -155,7 +155,7 @@ mmsClient_parseWriteResponse(ByteBuffer* message, int32_t bufPos, MmsError* mmsE
 
         bufPos = BerDecoder_decodeLength(buf, &length, bufPos, size);
 
-        if (bufPos == -1) {
+        if (bufPos < 0) {
             *mmsError = MMS_ERROR_PARSING_RESPONSE;
             retVal =  DATA_ACCESS_ERROR_UNKNOWN;
             goto exit_function;
@@ -171,7 +171,7 @@ mmsClient_parseWriteResponse(ByteBuffer* message, int32_t bufPos, MmsError* mmsE
         if (tag == 0x80) {
             bufPos = BerDecoder_decodeLength(buf, &length, bufPos, size);
 
-            if (bufPos == -1) {
+            if (bufPos < 0) {
                 *mmsError = MMS_ERROR_PARSING_RESPONSE;
                 retVal =  DATA_ACCESS_ERROR_UNKNOWN;
                 goto exit_function;

src/mms/iso_mms/server/mms_access_result.c

@@ -107,9 +107,8 @@ getNumberOfElements(uint8_t* buffer, int bufPos, int elementLength)
 
          bufPos = BerDecoder_decodeLength(buffer, &elementLength, bufPos, elementEndBufPos);
 
-         if ((bufPos < 0) || (bufPos + elementLength > elementEndBufPos)) {
+         if (bufPos < 0)
              goto exit_with_error;
-         }
 
          switch (tag) {
          case 0x80: /* reserved for access result */
@@ -164,7 +163,7 @@ MmsValue_decodeMmsData(uint8_t* buffer, int bufPos, int bufferLength, int* endBu
 
     bufPos = BerDecoder_decodeLength(buffer, &dataLength, bufPos, dataEndBufPos);
 
-    if (bufPos + dataLength > dataEndBufPos)
+    if (bufPos < 0)
         goto exit_with_error;
 
     switch (tag) {
@@ -188,7 +187,7 @@ MmsValue_decodeMmsData(uint8_t* buffer, int bufPos, int bufferLength, int* endBu
 
             int newBufPos = BerDecoder_decodeLength(buffer, &elementLength, bufPos + 1, dataEndBufPos);
 
-            if (newBufPos == -1)
+            if (newBufPos < 0)
                 goto exit_with_error;
 
             MmsValue* elementValue = MmsValue_decodeMmsData(buffer, bufPos, dataLength, NULL);

src/mms/iso_mms/server/mms_file_service.c

@@ -318,8 +318,6 @@ mmsServer_handleFileOpenRequest(
 
         if (bufPos < 0) goto exit_reject_invalid_pdu;
 
-        if (bufPos + length > maxBufPos) goto exit_reject_invalid_pdu;
-
         switch(tag) {
         case 0xa0: /* filename */
 
@@ -656,8 +654,6 @@ mmsServer_handleObtainFileRequest(
 
         if (bufPos < 0) goto exit_reject_invalid_pdu;
 
-        if (bufPos + length > maxBufPos) goto exit_reject_invalid_pdu;
-
         switch(tag) {
 
         case 0xa1: /* source filename */
@@ -1079,7 +1075,7 @@ mmsServer_handleFileRenameRequest(
 
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos < 0) || (bufPos + length > maxBufPos)) {
+        if (bufPos < 0) {
             mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
             return;
         }
@@ -1163,7 +1159,7 @@ mmsServer_handleFileDirectoryRequest(
 
         bufPos = BerDecoder_decodeLength(buffer, &length, bufPos, maxBufPos);
 
-        if ((bufPos < 0) || (bufPos + length > maxBufPos)) {
+        if (bufPos < 0) {
             mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
             return;
         }

src/mms/iso_mms/server/mms_journal_service.c

@@ -202,20 +202,20 @@ parseStringWithMaxLength(char* filename, int maxLength, uint8_t* buffer, int* bu
     int length;
 
     if (tag != 0x1a) {
-      mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
+        mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
-      return false;
+        return false;
     }
 
     *bufPos = BerDecoder_decodeLength(buffer, &length, *bufPos, maxBufPos);
 
-    if (*bufPos < 0)  {
+    if (*bufPos < 0) {
-      mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
+        mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
-      return false;
+        return false;
     }
 
     if (length > maxLength) {
-      mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_REQUEST_INVALID_ARGUMENT, response);
+        mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_REQUEST_INVALID_ARGUMENT, response);
-      return false;
+        return false;
     }
 
     memcpy(filename, buffer + *bufPos, length);

src/mms/iso_mms/server/mms_server_connection.c

@@ -161,11 +161,6 @@ handleConfirmedRequestPdu(
             return;
         }
 
-        if (bufPos + length > maxBufPos) {
-            mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response);
-            return;
-        }
-
         if (extendedTag) {
             switch (tag)
             {

src/sampled_values/sv_subscriber.c

@@ -403,19 +403,7 @@ parseSVPayload(SVReceiver self, SVSubscriber subscriber, uint8_t* buffer, int ap
             uint8_t tag = buffer[bufPos++];
 
             bufPos = BerDecoder_decodeLength(buffer, &elementLength, bufPos, svEnd);
-            if (bufPos < 0) {
+            if (bufPos < 0)
-                if (DEBUG_SV_SUBSCRIBER) printf("SV_SUBSCRIBER: Malformed message: failed to decode BER length tag!\n");
-                return;
-            }
-
-            if (bufPos + elementLength > apduLength) {
-                if (DEBUG_SV_SUBSCRIBER)
-                    printf("SV_SUBSCRIBER: Malformed message: sub element is too large!\n");
-
-                goto exit_error;
-            }
-
-            if (bufPos == -1)
                 goto exit_error;
 
             switch(tag) {