Vulnerability: An issue was discovered in libmodbus before 3.0.7 and 3.1.x before 3.1.5. There is an out-of-bounds read for the MODBUS_FC_WRITE_MULTIPLE_COILS case, aka VD-1302.
Vulnerability ID: CVE-2019-14462
CWE: CWE-125
Severity score: 4.9
Project: https://github.com/stephane/libmodbus
Published: 2019-07-31
File: src/modbus.c
Commit message: Fix VD-1301 and VD-1302 vulnerabilities

This patch was contributed by Maor Vermucht and Or Peles from
VDOO Connected Trust.
Commit author: Stéphane Raimbault

Vulnerable version (before fix):
 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 mapping_address = address - mb_mapping->start_bits;          if (nb < 1 || MODBUS_MAX_WRITE_BITS < 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. */ 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 mapping_address = address - mb_mapping->start_registers;          if (nb < 1 || MODBUS_MAX_WRITE_REGISTERS < 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",

Fixed version (after fix):
 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 || 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. */ 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 || 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",

Patch:
patch: @@ -839,9 +839,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. */
@@ -870,9 +871,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",
path: src/modbus.c