From 2551e034314bb9b0b666678fee1527e3b1009a73 Mon Sep 17 00:00:00 2001 From: Michael Heimpold Date: Sat, 8 Jan 2022 20:00:50 +0100 Subject: [PATCH 1/2] modbus_reply: fix copy & paste error in sanity check (fixes #614) While handling MODBUS_FC_WRITE_AND_READ_REGISTERS, both address offsets must be checked, i.e. the read and the write address must be within the mapping range. At the moment, only the read address was considered, it looks like a simple copy and paste error, so let's fix it. Signed-off-by: Michael Heimpold --- src/libmodbus/modbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmodbus/modbus.c b/src/libmodbus/modbus.c index 17e36e1..0123e8e 100644 --- a/src/libmodbus/modbus.c +++ b/src/libmodbus/modbus.c @@ -1017,7 +1017,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, nb_write, nb, MODBUS_MAX_WR_WRITE_REGISTERS, MODBUS_MAX_WR_READ_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_registers || - mapping_address < 0 || + mapping_address_write < 0 || (mapping_address_write + nb_write) > mb_mapping->nb_registers) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, From e19543e083acb8554b3bdd8f7c8ee4f6ea271957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 26 Jul 2019 16:00:06 +0200 Subject: [PATCH 2/2] Fix VD-1301 and VD-1302 vulnerabilities This patch was contributed by Maor Vermucht and Or Peles from VDOO Connected Trust. --- src/libmodbus/modbus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libmodbus/modbus.c b/src/libmodbus/modbus.c index 0123e8e..3afeb3b 100644 --- a/src/libmodbus/modbus.c +++ b/src/libmodbus/modbus.c @@ -897,9 +897,10 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_COILS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; + int nb_bits = req[offset + 5]; int mapping_address = address - mb_mapping->start_bits; - if (nb < 1 || MODBUS_MAX_WRITE_BITS < nb) { + if (nb < 1 || MODBUS_MAX_WRITE_BITS < nb || nb_bits * 8 < nb) { /* May be the indication has been truncated on reading because of * invalid address (eg. nb is 0 but the request contains values to * write) so it's necessary to flush. */ @@ -928,9 +929,10 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; + int nb_bytes = req[offset + 5]; int mapping_address = address - mb_mapping->start_registers; - if (nb < 1 || MODBUS_MAX_WRITE_REGISTERS < nb) { + if (nb < 1 || MODBUS_MAX_WRITE_REGISTERS < nb || nb_bytes * 8 < nb) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, "Illegal number of values %d in write_registers (max %d)\n",