Adjust modbus register array parsing.

For modbus message types that include variable amount of register values
(uint16[]), setting a &length attribute without an explicit array size
could trigger a parsing assertion since it allows for the "element" data
pointer to travel past the "end of data" (e.g. when &length is odd).
This is changed to now give both an array size and &length to earlier
terminate the parsing of elements before the assert is checked and
so a single out-of-bound check can be done for the entire array
(leaving off &length causes an out-of-bound check for each element).

Added another parameter to modbus events that carry register arrays to
the script-layer which indicates the associated byte count from the
message (allowing for invalid values to be detected):

    modbus_read_holding_registers_response
    modbus_read_input_registers_response
    modbus_write_multiple_registers_request
    modbus_read_write_multiple_registers_request
    modbus_read_write_multiple_registers_response
    modbus_read_fifo_queue_response
This commit is contained in:
Jon Siwek 2012-11-12 16:40:16 -06:00
parent defed7b6f3
commit c911d03c30
9 changed files with 79 additions and 22 deletions

View file

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

View file

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

Binary file not shown.

View file

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

View file

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