aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

modbus: fix heap-buffer-overflow in Modbus parser

Browse Source 
Modbus parser does not check length to extract/read data (read or write address,
quantity of data, etc.) that should be present.

In case of malformated data (invalid length in header), Modbus parser reads data
over the input data length.

Add check before extracting/reading data from input buffer to avoid head buffer
overflow.
pull/1509/head
DIALLO David 10 years ago
parent 07efec550d
commit 0a4fd39f9c
2 changed files with 249 additions and 109 deletions
Show Stats Download Patch File Download Diff File

274
src/app-layer-modbus.c
Unescape Escape View File

@ -385,15 +385,23 @@ void ModbusStateTxFree(void *state, uint64_t tx_id) {
* *
* \param res Pointer to the result * \param res Pointer to the result
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusExtractUint8(uint8_t *res, static int ModbusExtractUint8(ModbusState *modbus,
uint8_t *res,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) { uint16_t *offset) {
SCEnter(); SCEnter();
if (input_len < (uint32_t) (*offset + sizeof(uint8_t))) {
ModbusSetEvent(modbus, MODBUS_DECODER_EVENT_INVALID_LENGTH);
SCReturnInt(-1);
}
*res = *(input + *offset); *res = *(input + *offset);
*offset += sizeof(uint8_t); *offset += sizeof(uint8_t);
SCReturn; SCReturnInt(0);
} }
/** \internal /** \internal
@ -401,15 +409,23 @@ static void ModbusExtractUint8(uint8_t *res,
* *
* \param res Pointer to the result * \param res Pointer to the result
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusExtractUint16(uint16_t *res, static int ModbusExtractUint16(ModbusState *modbus,
uint16_t *res,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) { uint16_t *offset) {
SCEnter(); SCEnter();
if (input_len < (uint32_t) (*offset + sizeof(uint16_t))) {
ModbusSetEvent(modbus, MODBUS_DECODER_EVENT_INVALID_LENGTH);
SCReturnInt(-1);
}
ByteExtractUint16(res, BYTE_BIG_ENDIAN, sizeof(uint16_t), (const uint8_t *) (input + *offset)); ByteExtractUint16(res, BYTE_BIG_ENDIAN, sizeof(uint16_t), (const uint8_t *) (input + *offset));
*offset += sizeof(uint16_t); *offset += sizeof(uint16_t);
SCReturn; SCReturnInt(0);
} }
/** \internal /** \internal
@ -464,18 +480,21 @@ static void ModbusCheckHeader(ModbusState *modbus,
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusExceptionResponse(ModbusTransaction *tx, static void ModbusExceptionResponse(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
uint8_t exception; uint8_t exception;
/* Exception code (1 byte) */ /* Exception code (1 byte) */
ModbusExtractUint8(&exception, input, offset); if (ModbusExtractUint8(modbus, &exception, input, input_len, offset))
SCReturn;
switch (exception) { switch (exception) {
case MODBUS_ERROR_CODE_ILLEGAL_FUNCTION: case MODBUS_ERROR_CODE_ILLEGAL_FUNCTION:
@ -514,11 +533,13 @@ static void ModbusExceptionResponse(ModbusTransaction *tx,
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusParseReadRequest(ModbusTransaction *tx, static void ModbusParseReadRequest(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
@ -526,10 +547,12 @@ static void ModbusParseReadRequest(ModbusTransaction *tx,
uint8_t type = tx->type; uint8_t type = tx->type;
/* Starting Address (2 bytes) */ /* Starting Address (2 bytes) */
ModbusExtractUint16(&(tx->read.address), input, offset); if (ModbusExtractUint16(modbus, &(tx->read.address), input, input_len, offset))
goto end;
/* Quantity (2 bytes) */ /* Quantity (2 bytes) */
ModbusExtractUint16(&(tx->read.quantity), input, offset); if (ModbusExtractUint16(modbus, &(tx->read.quantity), input, input_len, offset))
goto end;
quantity = tx->read.quantity; quantity = tx->read.quantity;
/* Check Quantity range */ /* Check Quantity range */
@ -563,18 +586,21 @@ end:
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusParseReadResponse(ModbusTransaction *tx, static void ModbusParseReadResponse(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
uint8_t count; uint8_t count;
/* Count (1 bytes) */ /* Count (1 bytes) */
ModbusExtractUint8(&count, input, offset); if (ModbusExtractUint8(modbus, &count, input, input_len, offset))
goto end;
/* Check Count range and value according to the request */ /* Check Count range and value according to the request */
if ((tx->type) & MODBUS_TYP_BIT_ACCESS_MASK) { if ((tx->type) & MODBUS_TYP_BIT_ACCESS_MASK) {
@ -608,6 +634,7 @@ end:
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
* *
* \retval On success returns 0 or on failure returns -1. * \retval On success returns 0 or on failure returns -1.
@ -615,6 +642,7 @@ end:
static int ModbusParseWriteRequest(ModbusTransaction *tx, static int ModbusParseWriteRequest(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
@ -624,7 +652,8 @@ static int ModbusParseWriteRequest(ModbusTransaction *tx,
int i = 0; int i = 0;
/* Starting/Output/Register Address (2 bytes) */ /* Starting/Output/Register Address (2 bytes) */
ModbusExtractUint16(&(tx->write.address), input, offset); if (ModbusExtractUint16(modbus, &(tx->write.address), input, input_len, offset))
goto end;
if (type & MODBUS_TYP_SINGLE) { if (type & MODBUS_TYP_SINGLE) {
/* The length of Write Single Coil (code 5) and */ /* The length of Write Single Coil (code 5) and */
@ -634,11 +663,13 @@ static int ModbusParseWriteRequest(ModbusTransaction *tx,
goto end; goto end;
} else if (type & MODBUS_TYP_MULTIPLE) { } else if (type & MODBUS_TYP_MULTIPLE) {
/* Quantity (2 bytes) */ /* Quantity (2 bytes) */
ModbusExtractUint16(&quantity, input, offset); if (ModbusExtractUint16(modbus, &quantity, input, input_len, offset))
goto end;
tx->write.quantity = quantity; tx->write.quantity = quantity;
/* Count (1 bytes) */ /* Count (1 bytes) */
ModbusExtractUint8(&count, input, offset); if (ModbusExtractUint8(modbus, &count, input, input_len, offset))
goto end;
tx->write.count = count; tx->write.count = count;
if (type & MODBUS_TYP_BIT_ACCESS_MASK) { if (type & MODBUS_TYP_BIT_ACCESS_MASK) {
@ -691,7 +722,8 @@ static int ModbusParseWriteRequest(ModbusTransaction *tx,
if (type & MODBUS_TYP_SINGLE) { if (type & MODBUS_TYP_SINGLE) {
/* Outputs value (2 bytes) */ /* Outputs value (2 bytes) */
ModbusExtractUint16(&word, input, offset); if (ModbusExtractUint16(modbus, &word, input, input_len, offset))
goto end;
tx->data[i] = word; tx->data[i] = word;
if ((word != 0x00) && (word != 0xFF00)) if ((word != 0x00) && (word != 0xFF00))
@ -699,7 +731,8 @@ static int ModbusParseWriteRequest(ModbusTransaction *tx,
} else { } else {
for (i = 0; i < count; i++) { for (i = 0; i < count; i++) {
/* Outputs value (1 byte) */ /* Outputs value (1 byte) */
ModbusExtractUint8(&byte, input, offset); if (ModbusExtractUint8(modbus, &byte, input, input_len, offset))
goto end;
tx->data[i] = (uint16_t) byte; tx->data[i] = (uint16_t) byte;
} }
} }
@ -711,7 +744,8 @@ static int ModbusParseWriteRequest(ModbusTransaction *tx,
for (i = 0; i < quantity; i++) { for (i = 0; i < quantity; i++) {
/* Outputs/Registers value (2 bytes) */ /* Outputs/Registers value (2 bytes) */
ModbusExtractUint16(&word, input, offset); if (ModbusExtractUint16(modbus, &word, input, input_len, offset))
goto end;
tx->data[i] = word; tx->data[i] = word;
} }
} }
@ -729,11 +763,13 @@ end:
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusParseWriteResponse(ModbusTransaction *tx, static void ModbusParseWriteResponse(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
@ -741,21 +777,24 @@ static void ModbusParseWriteResponse(ModbusTransaction *tx,
uint8_t type = tx->type; uint8_t type = tx->type;
/* Starting Address (2 bytes) */ /* Starting Address (2 bytes) */
ModbusExtractUint16(&address, input, offset); if (ModbusExtractUint16(modbus, &address, input, input_len, offset))
goto end;
if (address != tx->write.address) if (address != tx->write.address)
goto error; goto error;
if (type & MODBUS_TYP_SINGLE) { if (type & MODBUS_TYP_SINGLE) {
/* Outputs/Registers value (2 bytes) */ /* Outputs/Registers value (2 bytes) */
ModbusExtractUint16(&word, input, offset); if (ModbusExtractUint16(modbus, &word, input, input_len, offset))
goto end;
/* Check with Outputs/Registers from request */ /* Check with Outputs/Registers from request */
if (word != tx->data[0]) if (word != tx->data[0])
goto error; goto error;
} else if (type & MODBUS_TYP_MULTIPLE) { } else if (type & MODBUS_TYP_MULTIPLE) {
/* Quantity (2 bytes) */ /* Quantity (2 bytes) */
ModbusExtractUint16(&quantity, input, offset); if (ModbusExtractUint16(modbus, &quantity, input, input_len, offset))
goto end;
/* Check Quantity range */ /* Check Quantity range */
if (type & MODBUS_TYP_BIT_ACCESS_MASK) { if (type & MODBUS_TYP_BIT_ACCESS_MASK) {
@ -773,14 +812,15 @@ static void ModbusParseWriteResponse(ModbusTransaction *tx,
goto error; goto error;
} else { } else {
/* And_Mask value (2 bytes) */ /* And_Mask value (2 bytes) */
ModbusExtractUint16(&word, input, offset); if (ModbusExtractUint16(modbus, &word, input, input_len, offset))
goto end;
/* Check And_Mask value according to the request */ /* Check And_Mask value according to the request */
if (word != tx->data[0]) if (word != tx->data[0])
goto error; goto error;
/* And_Or_Mask value (2 bytes) */ /* And_Or_Mask value (2 bytes) */
ModbusExtractUint16(&word, input, offset); if (ModbusExtractUint16(modbus, &word, input, input_len, offset))
/* Check Or_Mask value according to the request */ /* Check Or_Mask value according to the request */
if (word != tx->data[1]) if (word != tx->data[1])
@ -811,6 +851,7 @@ end:
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer to Modbus state structure * \param modbus Pointer to Modbus state structure
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
* *
* \retval Reserved category function returns 1 otherwise returns 0. * \retval Reserved category function returns 1 otherwise returns 0.
@ -818,16 +859,19 @@ end:
static int ModbusParseDiagnosticRequest(ModbusTransaction *tx, static int ModbusParseDiagnosticRequest(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint32_t input_len,
uint16_t *offset) uint16_t *offset)
{ {
SCEnter(); SCEnter();
uint16_t data; uint16_t data;
/* Sub-function (2 bytes) */ /* Sub-function (2 bytes) */
ModbusExtractUint16(&(tx->subFunction), input, offset); if (ModbusExtractUint16(modbus, &(tx->subFunction), input, input_len, offset))
goto end;
/* Data (2 bytes) */ /* Data (2 bytes) */
ModbusExtractUint16(&data, input, offset); if (ModbusExtractUint16(modbus, &data, input, input_len, offset))
goto end;
if (tx->subFunction != MODBUS_SUBFUNC_QUERY_DATA) { if (tx->subFunction != MODBUS_SUBFUNC_QUERY_DATA) {
switch (tx->subFunction) { switch (tx->subFunction) {
@ -908,20 +952,22 @@ static ModbusFunctionCodeRange modbusFunctionCodeRanges[] = {
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param ModbusPdu Pointer the Modbus PDU state in which the value to be stored * \param ModbusPdu Pointer the Modbus PDU state in which the value to be stored
* \param input Pointer the received input data * \param input Pointer the received input data
* \param offset Offset of the received input data pointer * \param input_len Length of the received input data
*/ */
static void ModbusParseRequestPDU(ModbusTransaction *tx, static void ModbusParseRequestPDU(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint16_t *offset) uint32_t input_len)
{ {
SCEnter(); SCEnter();
uint16_t offset = (uint16_t) sizeof(ModbusHeader);
uint8_t count; uint8_t count;
int i = 0; int i = 0;
/* Standard function codes used on MODBUS application layer protocol (1 byte) */ /* Standard function codes used on MODBUS application layer protocol (1 byte) */
ModbusExtractUint8(&(tx->function), input, offset); if (ModbusExtractUint8(modbus, &(tx->function), input, input_len, &offset))
goto end;
/* Set default function code category */ /* Set default function code category */
tx->category = MODBUS_CAT_NONE; tx->category = MODBUS_CAT_NONE;
@ -987,14 +1033,15 @@ static void ModbusParseRequestPDU(ModbusTransaction *tx,
case MODBUS_FUNC_READFILERECORD: case MODBUS_FUNC_READFILERECORD:
case MODBUS_FUNC_WRITEFILERECORD: case MODBUS_FUNC_WRITEFILERECORD:
/* Count/length (1 bytes) */ /* Count/length (1 bytes) */
ModbusExtractUint8(&count, input, offset); if (ModbusExtractUint8(modbus, &count, input, input_len, &offset))
goto end;
/* Modbus Application Protocol Specification V1.1b3 6.14 and 6.15 */ /* Modbus Application Protocol Specification V1.1b3 6.14 and 6.15 */
ModbusCheckHeaderLength(modbus, tx->length, 2 + count); ModbusCheckHeaderLength(modbus, tx->length, 2 + count);
break; break;
case MODBUS_FUNC_DIAGNOSTIC: case MODBUS_FUNC_DIAGNOSTIC:
if(ModbusParseDiagnosticRequest(tx, modbus, input, offset)) if(ModbusParseDiagnosticRequest(tx, modbus, input, input_len, &offset))
goto end; goto end;
break; break;
@ -1013,7 +1060,8 @@ static void ModbusParseRequestPDU(ModbusTransaction *tx,
case MODBUS_FUNC_ENCAPINTTRANS: case MODBUS_FUNC_ENCAPINTTRANS:
/* MEI type (1 byte) */ /* MEI type (1 byte) */
ModbusExtractUint8(&(tx->mei), input, offset); if (ModbusExtractUint8(modbus, &(tx->mei), input, input_len, &offset))
goto end;
if (tx->mei == MODBUS_MEI_ENCAPINTTRANS_READ) { if (tx->mei == MODBUS_MEI_ENCAPINTTRANS_READ) {
/* Modbus Application Protocol Specification V1.1b3 6.21 */ /* Modbus Application Protocol Specification V1.1b3 6.21 */
@ -1045,10 +1093,10 @@ static void ModbusParseRequestPDU(ModbusTransaction *tx,
tx->category = MODBUS_CAT_PUBLIC_ASSIGNED; tx->category = MODBUS_CAT_PUBLIC_ASSIGNED;
if (tx->type & MODBUS_TYP_READ) if (tx->type & MODBUS_TYP_READ)
ModbusParseReadRequest(tx, modbus, input, offset); ModbusParseReadRequest(tx, modbus, input, input_len, &offset);
if (tx->type & MODBUS_TYP_WRITE) if (tx->type & MODBUS_TYP_WRITE)
ModbusParseWriteRequest(tx, modbus, input, offset); ModbusParseWriteRequest(tx, modbus, input, input_len, &offset);
end: end:
SCReturn; SCReturn;
@ -1060,18 +1108,21 @@ end:
* \param tx Pointer to Modbus Transaction structure * \param tx Pointer to Modbus Transaction structure
* \param modbus Pointer the Modbus PDU state in which the value to be stored * \param modbus Pointer the Modbus PDU state in which the value to be stored
* \param input Pointer the received input data * \param input Pointer the received input data
* \param input_len Length of the received input data
* \param offset Offset of the received input data pointer * \param offset Offset of the received input data pointer
*/ */
static void ModbusParseResponsePDU(ModbusTransaction *tx, static void ModbusParseResponsePDU(ModbusTransaction *tx,
ModbusState *modbus, ModbusState *modbus,
uint8_t *input, uint8_t *input,
uint16_t *offset) uint32_t input_len)
{ {
SCEnter(); SCEnter();
uint16_t offset = (uint16_t) sizeof(ModbusHeader);
uint8_t count, error = FALSE, function, mei; uint8_t count, error = FALSE, function, mei;
/* Standard function codes used on MODBUS application layer protocol (1 byte) */ /* Standard function codes used on MODBUS application layer protocol (1 byte) */
ModbusExtractUint8(&function, input, offset); if (ModbusExtractUint8(modbus, &function, input, input_len, &offset))
goto end;
/* Check if response is error */ /* Check if response is error */
if(function & MODBUS_FUNC_ERRORMASK) { if(function & MODBUS_FUNC_ERRORMASK) {
@ -1082,7 +1133,7 @@ static void ModbusParseResponsePDU(ModbusTransaction *tx,
if (tx->category == MODBUS_CAT_PUBLIC_ASSIGNED) { if (tx->category == MODBUS_CAT_PUBLIC_ASSIGNED) {
/* Check if response is error. */ /* Check if response is error. */
if (error) { if (error) {
ModbusExceptionResponse(tx, modbus, input, offset); ModbusExceptionResponse(tx, modbus, input, input_len, &offset);
} else { } else {
switch(function) { switch(function) {
case MODBUS_FUNC_READEXCSTATUS: case MODBUS_FUNC_READEXCSTATUS:
@ -1098,7 +1149,8 @@ static void ModbusParseResponsePDU(ModbusTransaction *tx,
case MODBUS_FUNC_READFILERECORD: case MODBUS_FUNC_READFILERECORD:
case MODBUS_FUNC_WRITEFILERECORD: case MODBUS_FUNC_WRITEFILERECORD:
/* Count/length (1 bytes) */ /* Count/length (1 bytes) */
ModbusExtractUint8(&count, input, offset); if (ModbusExtractUint8(modbus, &count, input, input_len, &offset))
goto end;
/* Modbus Application Protocol Specification V1.1b3 6.14 and 6.15 */ /* Modbus Application Protocol Specification V1.1b3 6.14 and 6.15 */
ModbusCheckHeaderLength(modbus, tx->length, 2 + count); ModbusCheckHeaderLength(modbus, tx->length, 2 + count);
@ -1106,7 +1158,8 @@ static void ModbusParseResponsePDU(ModbusTransaction *tx,
case MODBUS_FUNC_ENCAPINTTRANS: case MODBUS_FUNC_ENCAPINTTRANS:
/* MEI type (1 byte) */ /* MEI type (1 byte) */
ModbusExtractUint8(&mei, input, offset); if (ModbusExtractUint8(modbus, &mei, input, input_len, &offset))
goto end;
if (mei != tx->mei) if (mei != tx->mei)
ModbusSetEvent(modbus, MODBUS_DECODER_EVENT_VALUE_MISMATCH); ModbusSetEvent(modbus, MODBUS_DECODER_EVENT_VALUE_MISMATCH);
@ -1114,10 +1167,10 @@ static void ModbusParseResponsePDU(ModbusTransaction *tx,
} }
if (tx->type & MODBUS_TYP_READ) if (tx->type & MODBUS_TYP_READ)
ModbusParseReadResponse(tx, modbus, input, offset); ModbusParseReadResponse(tx, modbus, input, input_len, &offset);
/* Read/Write response contents none write response part */ /* Read/Write response contents none write response part */
else if (tx->type & MODBUS_TYP_WRITE) else if (tx->type & MODBUS_TYP_WRITE)
ModbusParseWriteResponse(tx, modbus, input, offset); ModbusParseWriteResponse(tx, modbus, input, input_len, &offset);
} }
} }
@ -1130,26 +1183,26 @@ end:
* *
* \param header Pointer the Modbus header state in which the value to be stored * \param header Pointer the Modbus header state in which the value to be stored
* \param input Pointer the received input data * \param input Pointer the received input data
* \param offset Offset of the received input data pointer
*/ */
static void ModbusParseHeader(ModbusHeader *header, static int ModbusParseHeader(ModbusState *modbus,
ModbusHeader *header,
uint8_t *input, uint8_t *input,
uint16_t *offset) uint32_t input_len)
{ {
SCEnter(); SCEnter();
/* Transaction Identifier (2 bytes) */ uint16_t offset = 0;
ModbusExtractUint16(&(header->transactionId), input, offset);
/* Transaction Identifier (2 bytes) */
if (ModbusExtractUint16(modbus, &(header->transactionId), input, input_len, &offset) ||
/* Protocol Identifier (2 bytes) */ /* Protocol Identifier (2 bytes) */
ModbusExtractUint16(&(header->protocolId), input, offset); ModbusExtractUint16(modbus, &(header->protocolId), input, input_len, &offset) ||
/* Length (2 bytes) */ /* Length (2 bytes) */
ModbusExtractUint16(&(header->length), input, offset); ModbusExtractUint16(modbus, &(header->length), input, input_len, &offset) ||
/* Unit Identifier (1 byte) */ /* Unit Identifier (1 byte) */
ModbusExtractUint8(&(header->unitId), input, offset); ModbusExtractUint8(modbus, &(header->unitId), input, input_len, &offset))
SCReturnInt(-1);
SCReturn; SCReturnInt(0);
} }
/** \internal /** \internal
@ -1175,18 +1228,14 @@ static int ModbusParseRequest(Flow *f,
ModbusHeader header; ModbusHeader header;
while (input_len > 0) { while (input_len > 0) {
uint32_t adu_len; uint32_t adu_len = input_len;
uint16_t offset = 0; uint8_t *adu = input;
uint8_t *ptr = input;
/* Modbus header is 7 bytes long */
if (input_len < (uint32_t) sizeof(ModbusHeader))
SCReturnInt(0);
/* Extract MODBUS Header */ /* Extract MODBUS Header */
ModbusParseHeader(&header, ptr, &offset); if (ModbusParseHeader(modbus, &header, adu, adu_len))
SCReturnInt(0);
/* Compute ADU length. */ /* Update ADU length with length in Modbus header. */
adu_len = (uint32_t) sizeof(ModbusHeader) + (uint32_t) header.length - 1; adu_len = (uint32_t) sizeof(ModbusHeader) + (uint32_t) header.length - 1;
if (adu_len > input_len) if (adu_len > input_len)
SCReturnInt(0); SCReturnInt(0);
@ -1204,7 +1253,7 @@ static int ModbusParseRequest(Flow *f,
tx->length = header.length; tx->length = header.length;
/* Extract MODBUS PDU and fill Transaction Context */ /* Extract MODBUS PDU and fill Transaction Context */
ModbusParseRequestPDU(tx, modbus, ptr, &offset); ModbusParseRequestPDU(tx, modbus, adu, adu_len);
/* Update input line and remaining input length of the command */ /* Update input line and remaining input length of the command */
input += adu_len; input += adu_len;
@ -1236,18 +1285,14 @@ static int ModbusParseResponse(Flow *f,
ModbusTransaction *tx; ModbusTransaction *tx;
while (input_len > 0) { while (input_len > 0) {
uint32_t adu_len; uint32_t adu_len = input_len;
uint16_t offset = 0; uint8_t *adu = input;
uint8_t *ptr = input;
/* Modbus header is 7 bytes long */
if (input_len < (uint32_t) sizeof(ModbusHeader))
SCReturnInt(0);
/* Extract MODBUS Header */ /* Extract MODBUS Header */
ModbusParseHeader(&header, ptr, &offset); if (ModbusParseHeader(modbus, &header, adu, adu_len))
SCReturnInt(0);
/* Compute ADU length */ /* Update ADU length with length in Modbus header. */
adu_len = (uint32_t) sizeof(ModbusHeader) + (uint32_t) header.length - 1; adu_len = (uint32_t) sizeof(ModbusHeader) + (uint32_t) header.length - 1;
if (adu_len > input_len) if (adu_len > input_len)
SCReturnInt(0); SCReturnInt(0);
@ -1268,7 +1313,7 @@ static int ModbusParseResponse(Flow *f,
tx->length = header.length; tx->length = header.length;
/* Extract MODBUS PDU and fill Transaction Context */ /* Extract MODBUS PDU and fill Transaction Context */
ModbusParseResponsePDU(tx, modbus, ptr, &offset); ModbusParseResponsePDU(tx, modbus, adu, adu_len);
} }
/* Check and store MODBUS Header */ /* Check and store MODBUS Header */
@ -1565,6 +1610,13 @@ static uint8_t exceededLengthWriteMultipleRegistersReq[] = {
/* Quantity of Registers */ 0x7f, 0xf9, /* Quantity of Registers */ 0x7f, 0xf9,
/* Byte count */ 0xff}; /* Byte count */ 0xff};
static uint8_t invalidLengthPDUWriteMultipleRegistersReq[] = {
/* Transaction ID */ 0x00, 0x0A,
/* Protocol ID */ 0x00, 0x00,
/* Length */ 0x00, 0x02,
/* Unit ID */ 0x00,
/* Function code */ 0x10};
/** \test Send Modbus Read Coils request/response. */ /** \test Send Modbus Read Coils request/response. */
static int ModbusParserTest01(void) { static int ModbusParserTest01(void) {
AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc(); AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc();
@ -2500,6 +2552,93 @@ end:
UTHFreePackets(&p, 1); UTHFreePackets(&p, 1);
return result; return result;
} }
/** \test Send Modbus invalid PDU Length. */
static int ModbusParserTest12(void) {
AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc();
DetectEngineThreadCtx *det_ctx = NULL;
Flow f;
Packet *p = NULL;
Signature *s = NULL;
TcpSession ssn;
ThreadVars tv;
int result = 0;
memset(&tv, 0, sizeof(ThreadVars));
memset(&f, 0, sizeof(Flow));
memset(&ssn, 0, sizeof(TcpSession));
p = UTHBuildPacket(NULL, 0, IPPROTO_TCP);
FLOW_INITIALIZE(&f);
f.alproto = ALPROTO_MODBUS;
f.protoctx = (void *)&ssn;
f.proto = IPPROTO_TCP;
f.flags |= FLOW_IPV4;
p->flow = &f;
p->flags |= PKT_HAS_FLOW | PKT_STREAM_EST;
p->flowflags |= FLOW_PKT_TOSERVER | FLOW_PKT_ESTABLISHED;
StreamTcpInitConfig(TRUE);
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
if (de_ctx == NULL)
goto end;
de_ctx->flags |= DE_QUIET;
s = DetectEngineAppendSig(de_ctx, "alert modbus any any -> any any "
"(msg:\"Modbus invalid Length\"; "
"app-layer-event: "
"modbus.invalid_length; "
"sid:1;)");
if (s == NULL)
goto end;
SigGroupBuild(de_ctx);
DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);
SCMutexLock(&f.m);
int r = AppLayerParserParse(alp_tctx, &f, ALPROTO_MODBUS, STREAM_TOSERVER,
invalidLengthPDUWriteMultipleRegistersReq,
sizeof(invalidLengthPDUWriteMultipleRegistersReq));
if (r != 0) {
printf("toserver chunk 1 returned %" PRId32 ", expected 0: ", r);
SCMutexUnlock(&f.m);
goto end;
}
SCMutexUnlock(&f.m);
ModbusState *modbus_state = f.alstate;
if (modbus_state == NULL) {
printf("no modbus state: ");
goto end;
}
/* do detect */
SigMatchSignatures(&tv, de_ctx, det_ctx, p);
if (!PacketAlertCheck(p, 1)) {
printf("sid 1 didn't match. Should have matched: ");
goto end;
}
result = 1;
end:
SigGroupCleanup(de_ctx);
SigCleanSignatures(de_ctx);
DetectEngineThreadCtxDeinit(&tv, (void *)det_ctx);
DetectEngineCtxFree(de_ctx);
if (alp_tctx != NULL)
AppLayerParserThreadCtxFree(alp_tctx);
StreamTcpFreeConfig(TRUE);
FLOW_DESTROY(&f);
UTHFreePackets(&p, 1);
return result;
}
#endif /* UNITTESTS */ #endif /* UNITTESTS */
void ModbusParserRegisterTests(void) { void ModbusParserRegisterTests(void) {
@ -2515,5 +2654,6 @@ void ModbusParserRegisterTests(void) {
UtRegisterTest("ModbusParserTest09 - Modbus fragmentation - 1 ADU in 2 TCP packets", ModbusParserTest09, 1); UtRegisterTest("ModbusParserTest09 - Modbus fragmentation - 1 ADU in 2 TCP packets", ModbusParserTest09, 1);
UtRegisterTest("ModbusParserTest10 - Modbus fragmentation - 2 ADU in 1 TCP packet", ModbusParserTest10, 1); UtRegisterTest("ModbusParserTest10 - Modbus fragmentation - 2 ADU in 1 TCP packet", ModbusParserTest10, 1);
UtRegisterTest("ModbusParserTest11 - Modbus exceeded Length request", ModbusParserTest11, 1); UtRegisterTest("ModbusParserTest11 - Modbus exceeded Length request", ModbusParserTest11, 1);
UtRegisterTest("ModbusParserTest12 - Modbus invalid PDU Length", ModbusParserTest12, 1);
#endif /* UNITTESTS */ #endif /* UNITTESTS */
} }

2
src/detect-engine-modbus.c
Unescape Escape View File

@ -283,7 +283,7 @@ static uint8_t readCoilsReq[] = {/* Transaction ID */ 0x00, 0x00,
/* Example of a request to read input register 9 */ /* Example of a request to read input register 9 */
static uint8_t readInputsRegistersReq[] = {/* Transaction ID */ 0x00, 0x0A, static uint8_t readInputsRegistersReq[] = {/* Transaction ID */ 0x00, 0x0A,
/* Protocol ID */ 0x00, 0x00, /* Protocol ID */ 0x00, 0x00,
/* Length */ 0x00, 0x05, /* Length */ 0x00, 0x06,
/* Unit ID */ 0x00, /* Unit ID */ 0x00,
/* Function code */ 0x04, /* Function code */ 0x04,
/* Starting Address */ 0x00, 0x08, /* Starting Address */ 0x00, 0x08,

Write Preview
Loading…
Cancel
Save