diff --git a/scripts/policy/protocols/modbus/track-memmap.bro b/scripts/policy/protocols/modbus/track-memmap.bro index fc02d9b274..cc02ce9e6a 100644 --- a/scripts/policy/protocols/modbus/track-memmap.bro +++ b/scripts/policy/protocols/modbus/track-memmap.bro @@ -60,7 +60,7 @@ event modbus_read_holding_registers_request(c: connection, headers: ModbusHeader c$modbus$track_address = start_address+1; } -event modbus_read_holding_registers_response(c: connection, headers: ModbusHeaders, registers: ModbusRegisters) +event modbus_read_holding_registers_response(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters) { local slave = c$id$resp_h; diff --git a/src/event.bif b/src/event.bif index b965c26ae9..cc8acb1849 100644 --- a/src/event.bif +++ b/src/event.bif @@ -6623,8 +6623,10 @@ event modbus_read_holding_registers_request%(c: connection, headers: ModbusHeade ## ## headers: The headers for the modbus function. ## +## byte_count: The number of bytes in the message that comprise register values. +## ## registers: The register values returned from the device. -event modbus_read_holding_registers_response%(c: connection, headers: ModbusHeaders, registers: ModbusRegisters%); +event modbus_read_holding_registers_response%(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters%); ## Generated for a Modbus read input registers request. ## @@ -6643,8 +6645,10 @@ event modbus_read_input_registers_request%(c: connection, headers: ModbusHeaders ## ## headers: The headers for the modbus function. ## +## byte_count: The number of bytes in the message that comprise register values. +## ## registers: The register values returned from the device. -event modbus_read_input_registers_response%(c: connection, headers: ModbusHeaders, registers: ModbusRegisters%); +event modbus_read_input_registers_response%(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters%); ## Generated for a Modbus write single coil request. ## @@ -6720,8 +6724,10 @@ event modbus_write_multiple_coils_response%(c: connection, headers: ModbusHeader ## ## start_address: The memory address of the first register to be written. ## +## byte_count: The number of bytes in the message that comprise register values. +## ## registers: The values to be written to the registers. -event modbus_write_multiple_registers_request%(c: connection, headers: ModbusHeaders, start_address: count, registers: ModbusRegisters%); +event modbus_write_multiple_registers_request%(c: connection, headers: ModbusHeaders, start_address: count, byte_count: count, registers: ModbusRegisters%); ## Generated for a Modbus write multiple registers response. ## @@ -6812,8 +6818,10 @@ event modbus_mask_write_register_response%(c: connection, headers: ModbusHeaders ## ## write_start_address: The memory address of the first register to be written. ## +## write_byte_count: Number of bytes in message that comprise register values. +## ## write_registers: The values to be written to the registers. -event modbus_read_write_multiple_registers_request%(c: connection, headers: ModbusHeaders, read_start_address: count, read_quantity: count, write_start_address: count, write_registers: ModbusRegisters%); +event modbus_read_write_multiple_registers_request%(c: connection, headers: ModbusHeaders, read_start_address: count, read_quantity: count, write_start_address: count, write_byte_count: count, write_registers: ModbusRegisters%); ## Generated for a Modbus read/write multiple registers response. ## @@ -6821,8 +6829,10 @@ event modbus_read_write_multiple_registers_request%(c: connection, headers: Modb ## ## headers: The headers for the modbus function. ## +## byte_count: The number of bytes in the message that comprise register values. +## ## written_registers: The register values read from the registers specified in the request. -event modbus_read_write_multiple_registers_response%(c: connection, headers: ModbusHeaders, written_registers: ModbusRegisters%); +event modbus_read_write_multiple_registers_response%(c: connection, headers: ModbusHeaders, byte_count: count, written_registers: ModbusRegisters%); ## Generated for a Modbus read FIFO queue request. ## @@ -6839,8 +6849,10 @@ event modbus_read_fifo_queue_request%(c: connection, headers: ModbusHeaders, sta ## ## headers: The headers for the modbus function. ## +## byte_count: The number of bytes in the message that comprise register values. +## ## fifos: The register values read from the FIFO queue on the device. -event modbus_read_fifo_queue_response%(c: connection, headers: ModbusHeaders, fifos: ModbusRegisters%); +event modbus_read_fifo_queue_response%(c: connection, headers: ModbusHeaders, byte_count: count, fifos: ModbusRegisters%); ## Raised for informational messages reported via Bro's reporter framework. Such ## messages may be generated internally by the event engine and also by other diff --git a/src/modbus-analyzer.pac b/src/modbus-analyzer.pac index ad59a3471a..155da9647f 100644 --- a/src/modbus-analyzer.pac +++ b/src/modbus-analyzer.pac @@ -147,6 +147,7 @@ refine flow ModbusTCP_Flow += { BifEvent::generate_modbus_read_holding_registers_response(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), HeaderToBro(header), + ${message.byte_count}, t); } @@ -182,7 +183,9 @@ refine flow ModbusTCP_Flow += { BifEvent::generate_modbus_read_input_registers_response(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), - HeaderToBro(header), t); + HeaderToBro(header), + ${message.byte_count}, + t); } return true; @@ -318,7 +321,7 @@ refine flow ModbusTCP_Flow += { BifEvent::generate_modbus_write_multiple_registers_request(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), HeaderToBro(header), - ${message.start_address}, t); + ${message.start_address}, ${message.byte_count}, t); } return true; @@ -498,6 +501,7 @@ refine flow ModbusTCP_Flow += { ${message.read_start_address}, ${message.read_quantity}, ${message.write_start_address}, + ${message.write_byte_count}, t); } @@ -518,7 +522,9 @@ refine flow ModbusTCP_Flow += { BifEvent::generate_modbus_read_write_multiple_registers_response(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), - HeaderToBro(header), t); + HeaderToBro(header), + ${message.byte_count}, + t); } return true; @@ -553,7 +559,9 @@ refine flow ModbusTCP_Flow += { BifEvent::generate_modbus_read_fifo_queue_response(connection()->bro_analyzer(), connection()->bro_analyzer()->Conn(), - HeaderToBro(header), t); + HeaderToBro(header), + ${message.byte_count}, + t); } return true; diff --git a/src/modbus-protocol.pac b/src/modbus-protocol.pac index cef2626270..a79e4dccf5 100644 --- a/src/modbus-protocol.pac +++ b/src/modbus-protocol.pac @@ -192,7 +192,7 @@ type ReadHoldingRegistersRequest(header: ModbusTCP_TransportHeader) = record { # RESPONSE FC=3 type ReadHoldingRegistersResponse(header: ModbusTCP_TransportHeader) = record { byte_count: uint8; - registers: uint16[] &length=byte_count; + registers: uint16[byte_count/2] &length=byte_count; } &let { deliver: bool = $context.flow.deliver_ReadHoldingRegistersResponse(header, this); } &byteorder=bigendian; @@ -208,7 +208,7 @@ type ReadInputRegistersRequest(header: ModbusTCP_TransportHeader) = record { # RESPONSE FC=4 type ReadInputRegistersResponse(header: ModbusTCP_TransportHeader) = record { byte_count: uint8; - registers: uint16[] &length=byte_count; + registers: uint16[byte_count/2] &length=byte_count; } &let { deliver: bool = $context.flow.deliver_ReadInputRegistersResponse(header, this); } &byteorder=bigendian; @@ -303,7 +303,7 @@ type ReadFileRecordRequest(header: ModbusTCP_TransportHeader) = record { type FileRecordResponse = record { file_len: uint8 &check(file_len >= 0x07 && file_len <= 0xF5); ref_type: uint8 &check(ref_type == 6); - record_data: uint16[] &length=file_len; + record_data: uint16[file_len/2] &length=file_len; } &byteorder=bigendian; # RESPONSE FC=20 @@ -372,7 +372,7 @@ type ReadWriteMultipleRegistersRequest(header: ModbusTCP_TransportHeader) = reco # RESPONSE FC=23 type ReadWriteMultipleRegistersResponse(header: ModbusTCP_TransportHeader) = record { byte_count: uint8; - registers: uint16[] &length=byte_count; + registers: uint16[byte_count/2] &length=byte_count; } &let { deliver: bool = $context.flow.deliver_ReadWriteMultipleRegistersResponse(header, this); } &byteorder=bigendian; @@ -388,7 +388,7 @@ type ReadFIFOQueueRequest(header: ModbusTCP_TransportHeader) = record { type ReadFIFOQueueResponse(header: ModbusTCP_TransportHeader) = record { byte_count: uint16 &check(byte_count <= 62); fifo_count: uint16 &check(fifo_count <= 31); - register_data: uint16[fifo_count] &length=byte_count/2; + register_data: uint16[fifo_count] &length=fifo_count*2; } &let { deliver: bool = $context.flow.deliver_ReadFIFOQueueResponse(header, this); } &byteorder=bigendian; diff --git a/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/modbus.log b/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/modbus.log new file mode 100644 index 0000000000..85a3a6117c --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/modbus.log @@ -0,0 +1,12 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path modbus +#open 2012-11-12-21-51-15 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p func exception +#types time string addr port addr port string string +1342774775.305761 UWkUyAuUGXf 10.1.1.234 51411 10.10.5.104 502 READ_INPUT_REGISTERS - +1342775209.493066 arKYeMETxOg 10.1.1.234 51411 10.10.5.104 502 READ_INPUT_REGISTERS - +1342776371.617757 nQcgTWjvg4c 10.1.1.234 51411 10.10.5.104 502 READ_INPUT_REGISTERS - +#close 2012-11-12-21-51-15 diff --git a/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/output b/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/output new file mode 100644 index 0000000000..353f85d2ef --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.modbus.register_parsing/output @@ -0,0 +1,5 @@ +modbus_read_input_registers_request, [orig_h=10.1.1.234, orig_p=51411/tcp, resp_h=10.10.5.104, resp_p=502/tcp], [tid=1119, pid=0, uid=255, function_code=4], 900, 147 +modbus_read_input_registers_response, [orig_h=10.1.1.234, orig_p=51411/tcp, resp_h=10.10.5.104, resp_p=502/tcp], [tid=2606, pid=0, uid=255, function_code=4], [0, 0, 0, 0, 0, 0, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690], 100, 200 +modbus_read_input_registers_response, [orig_h=10.1.1.234, orig_p=51411/tcp, resp_h=10.10.5.104, resp_p=502/tcp], [tid=6714, pid=0, uid=255, function_code=4], [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3840, 0, 0, 31, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 37, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 64, 129 +modbus_read_input_registers_request, [orig_h=10.1.1.234, orig_p=51411/tcp, resp_h=10.10.5.104, resp_p=502/tcp], [tid=12993, pid=0, uid=255, function_code=4], 400, 100 +modbus_read_input_registers_response, [orig_h=10.1.1.234, orig_p=51411/tcp, resp_h=10.10.5.104, resp_p=502/tcp], [tid=17667, pid=0, uid=255, function_code=4], [49, 18012, 51, 42, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 49, 50, 51, 54324, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 69, 63, 64, 65, 66, 67, 68, 49, 189, 51, 52, 53, 54, 4151, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 136, 49, 50, 51, 212, 53, 54, 170, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690, 43690], 100, 200 diff --git a/testing/btest/Traces/modbus/fuzz-1011.trace b/testing/btest/Traces/modbus/fuzz-1011.trace new file mode 100644 index 0000000000..b1deea7109 Binary files /dev/null and b/testing/btest/Traces/modbus/fuzz-1011.trace differ diff --git a/testing/btest/scripts/base/protocols/modbus/events.bro b/testing/btest/scripts/base/protocols/modbus/events.bro index f648a0adde..6c47dc611a 100644 --- a/testing/btest/scripts/base/protocols/modbus/events.bro +++ b/testing/btest/scripts/base/protocols/modbus/events.bro @@ -41,7 +41,7 @@ event modbus_read_holding_registers_request(c: connection, headers: ModbusHeader print "modbus_read_holding_registers_request", c, headers, start_address, quantity; } -event modbus_read_holding_registers_response(c: connection, headers: ModbusHeaders, registers: ModbusRegisters) +event modbus_read_holding_registers_response(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters) { print "modbus_read_holding_registers_response", c, headers, registers; } @@ -51,7 +51,7 @@ event modbus_read_input_registers_request(c: connection, headers: ModbusHeaders, print "modbus_read_input_registers_request", c, headers, start_address, quantity; } -event modbus_read_input_registers_response(c: connection, headers: ModbusHeaders, registers: ModbusRegisters) +event modbus_read_input_registers_response(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters) { print "modbus_read_input_registers_response", c, headers, registers; } @@ -86,7 +86,7 @@ event modbus_write_multiple_coils_response(c: connection, headers: ModbusHeaders print "modbus_write_multiple_coils_response", c, headers, start_address, quantity; } -event modbus_write_multiple_registers_request(c: connection, headers: ModbusHeaders, start_address: count, registers: ModbusRegisters) +event modbus_write_multiple_registers_request(c: connection, headers: ModbusHeaders, start_address: count, byte_count: count, registers: ModbusRegisters) { print "modbus_write_multiple_registers_request", c, headers, start_address, registers; } @@ -126,12 +126,12 @@ event modbus_mask_write_register_response(c: connection, headers: ModbusHeaders, print "modbus_mask_write_register_response", c, headers, address, and_mask, or_mask; } -event modbus_read_write_multiple_registers_request(c: connection, headers: ModbusHeaders, read_start_address: count, read_quantity: count, write_start_address: count, write_registers: ModbusRegisters) +event modbus_read_write_multiple_registers_request(c: connection, headers: ModbusHeaders, read_start_address: count, read_quantity: count, write_start_address: count, write_byte_count: count, write_registers: ModbusRegisters) { print "modbus_read_write_multiple_registers_request", c, headers, read_start_address, read_quantity, write_start_address, write_registers; } -event modbus_read_write_multiple_registers_response(c: connection, headers: ModbusHeaders, written_registers: ModbusRegisters) +event modbus_read_write_multiple_registers_response(c: connection, headers: ModbusHeaders, byte_count: count, written_registers: ModbusRegisters) { print "modbus_read_write_multiple_registers_response", c, headers, written_registers; } @@ -141,7 +141,7 @@ event modbus_read_fifo_queue_request(c: connection, headers: ModbusHeaders, star print "modbus_read_fifo_queue_request", c, headers, start_address; } -event modbus_read_fifo_queue_response(c: connection, headers: ModbusHeaders, fifos: ModbusRegisters) +event modbus_read_fifo_queue_response(c: connection, headers: ModbusHeaders, byte_count: count, fifos: ModbusRegisters) { print "modbus_read_fifo_queue_response", c, headers, fifos; } diff --git a/testing/btest/scripts/base/protocols/modbus/register_parsing.bro b/testing/btest/scripts/base/protocols/modbus/register_parsing.bro new file mode 100644 index 0000000000..300dd75bfe --- /dev/null +++ b/testing/btest/scripts/base/protocols/modbus/register_parsing.bro @@ -0,0 +1,20 @@ +# @TEST-EXEC: bro -r $TRACES/modbus/fuzz-1011.trace %INPUT >output +# @TEST-EXEC: btest-diff modbus.log +# @TEST-EXEC: btest-diff output + +# modbus registers are 2-byte values. Many messages send a variable amount +# of register values, with the quantity being derived from a byte count value +# that is also sent. If the byte count value is invalid (e.g. an odd value +# might not be valid since registers must be 2-byte values), then the parser +# should not trigger any asserts, but the resulting event could indicate +# the strangeness (i.e. byte_count != 2*|registers|). + +event modbus_read_input_registers_request(c: connection, headers: ModbusHeaders, start_address: count, quantity: count) + { + print "modbus_read_input_registers_request", c$id, headers, start_address, quantity; + } + +event modbus_read_input_registers_response(c: connection, headers: ModbusHeaders, byte_count: count, registers: ModbusRegisters) + { + print "modbus_read_input_registers_response", c$id, headers, registers, |registers|, byte_count; + }