diff --git a/src/goose/goose_receiver.c b/src/goose/goose_receiver.c index a5139339..afae2565 100644 --- a/src/goose/goose_receiver.c +++ b/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; diff --git a/src/iec61850/server/mms_mapping/reporting.c b/src/iec61850/server/mms_mapping/reporting.c index 6fecb102..1742f315 100644 --- a/src/iec61850/server/mms_mapping/reporting.c +++ b/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) { diff --git a/src/mms/asn1/ber_decode.c b/src/mms/asn1/ber_decode.c index 32be3975..db6c8d91 100644 --- a/src/mms/asn1/ber_decode.c +++ b/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; diff --git a/src/mms/iso_mms/client/mms_client_connection.c b/src/mms/iso_mms/client/mms_client_connection.c index ad1ddfcf..33332131 100644 --- a/src/mms/iso_mms/client/mms_client_connection.c +++ b/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) { diff --git a/src/mms/iso_mms/client/mms_client_files.c b/src/mms/iso_mms/client/mms_client_files.c index 0df2c95d..c21bc0a4 100644 --- a/src/mms/iso_mms/client/mms_client_files.c +++ b/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; diff --git a/src/mms/iso_mms/client/mms_client_get_namelist.c b/src/mms/iso_mms/client/mms_client_get_namelist.c index 9234f49d..a7facd1d 100644 --- a/src/mms/iso_mms/client/mms_client_get_namelist.c +++ b/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 diff --git a/src/mms/iso_mms/client/mms_client_identify.c b/src/mms/iso_mms/client/mms_client_identify.c index b746a59b..c065a754 100644 --- a/src/mms/iso_mms/client/mms_client_identify.c +++ b/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]; diff --git a/src/mms/iso_mms/client/mms_client_initiate.c b/src/mms/iso_mms/client/mms_client_initiate.c index 2721f5a8..57288551 100644 --- a/src/mms/iso_mms/client/mms_client_initiate.c +++ b/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); diff --git a/src/mms/iso_mms/client/mms_client_journals.c b/src/mms/iso_mms/client/mms_client_journals.c index 6bb8fb13..c979fc17 100644 --- a/src/mms/iso_mms/client/mms_client_journals.c +++ b/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) { diff --git a/src/mms/iso_mms/client/mms_client_status.c b/src/mms/iso_mms/client/mms_client_status.c index 392fc665..0e19823c 100644 --- a/src/mms/iso_mms/client/mms_client_status.c +++ b/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; diff --git a/src/mms/iso_mms/client/mms_client_write.c b/src/mms/iso_mms/client/mms_client_write.c index d6a80a83..ec88c8a6 100644 --- a/src/mms/iso_mms/client/mms_client_write.c +++ b/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; diff --git a/src/mms/iso_mms/server/mms_access_result.c b/src/mms/iso_mms/server/mms_access_result.c index 1ed962f0..7c9957e3 100644 --- a/src/mms/iso_mms/server/mms_access_result.c +++ b/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); diff --git a/src/mms/iso_mms/server/mms_file_service.c b/src/mms/iso_mms/server/mms_file_service.c index 49e8d4b8..4e965060 100644 --- a/src/mms/iso_mms/server/mms_file_service.c +++ b/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; } diff --git a/src/mms/iso_mms/server/mms_journal_service.c b/src/mms/iso_mms/server/mms_journal_service.c index 0bbed87f..fb6339a6 100644 --- a/src/mms/iso_mms/server/mms_journal_service.c +++ b/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); - return false; + mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response); + return false; } *bufPos = BerDecoder_decodeLength(buffer, &length, *bufPos, maxBufPos); - if (*bufPos < 0) { - mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response); - return false; + if (*bufPos < 0) { + mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_INVALID_PDU, response); + return false; } if (length > maxLength) { - mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_REQUEST_INVALID_ARGUMENT, response); - return false; + mmsMsg_createMmsRejectPdu(&invokeId, MMS_ERROR_REJECT_REQUEST_INVALID_ARGUMENT, response); + return false; } memcpy(filename, buffer + *bufPos, length); diff --git a/src/mms/iso_mms/server/mms_server_connection.c b/src/mms/iso_mms/server/mms_server_connection.c index 8bb7b7e1..8c746bf2 100644 --- a/src/mms/iso_mms/server/mms_server_connection.c +++ b/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) { diff --git a/src/sampled_values/sv_subscriber.c b/src/sampled_values/sv_subscriber.c index c5a73fcd..1d49b39c 100644 --- a/src/sampled_values/sv_subscriber.c +++ b/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 (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) + if (bufPos < 0) goto exit_error; switch(tag) {