Check length passed to modbus_reply (write_register)

Related to df79a02.

Author: stephane

src/modbus.c

@@ -910,13 +910,14 @@ int modbus_reply(modbus_t *ctx,
         rsp_length = compute_response_length_from_request(ctx, (uint8_t *) req);
         if (rsp_length != req_length) {
             /* Bad use of modbus_reply */
-            rsp_length = response_exception(ctx,
-                                            &sft,
-                                            MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE,
-                                            rsp,
-                                            FALSE,
-                                            "Invalid request length (%d)\n",
-                                            req_length);
+            rsp_length =
+                response_exception(ctx,
+                                   &sft,
+                                   MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE,
+                                   rsp,
+                                   FALSE,
+                                   "Invalid request length used in modbus_reply (%d)\n",
+                                   req_length);
             break;
         }
 
@@ -948,13 +949,26 @@ int modbus_reply(modbus_t *ctx,
                                    FALSE,
                                    "Illegal data address 0x%0X in write_register\n",
                                    address);
-        } else {
-            int data = (req[offset + 3] << 8) + req[offset + 4];
+            break;
+        }
 
-            mb_mapping->tab_registers[mapping_address] = data;
-            memcpy(rsp, req, req_length);
-            rsp_length = req_length;
+        rsp_length = compute_response_length_from_request(ctx, (uint8_t *) req);
+        if (rsp_length != req_length) {
+            /* Bad use of modbus_reply */
+            rsp_length =
+                response_exception(ctx,
+                                   &sft,
+                                   MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE,
+                                   rsp,
+                                   FALSE,
+                                   "Invalid request length used in modbus_reply (%d)\n",
+                                   req_length);
+            break;
         }
+        int data = (req[offset + 3] << 8) + req[offset + 4];
+
+        mb_mapping->tab_registers[mapping_address] = data;
+        memcpy(rsp, req, rsp_length);
     } break;
     case MODBUS_FC_WRITE_MULTIPLE_COILS: {
         int nb = (req[offset + 3] << 8) + req[offset + 4];

tests/unit-test-client.c

@@ -499,13 +499,17 @@ int main(int argc, char *argv[])
 
     modbus_disable_quirks(ctx, MODBUS_QUIRK_MAX_SLAVE);
     rc = modbus_set_slave(ctx, old_slave);
-    ASSERT_TRUE(rc == 0, "Uanble to restore slave value")
+    ASSERT_TRUE(rc == 0, "Unable to restore slave value")
 
     /** BAD USE OF REPLY FUNCTION **/
     rc = modbus_write_bit(ctx, UT_BITS_ADDRESS_INVALID_REQUEST_LENGTH, ON);
     printf("* modbus_write_bit (triggers invalid reply): ");
     ASSERT_TRUE(rc == -1 && errno == EMBXILVAL, "");
 
+    rc = modbus_write_register(ctx, UT_REGISTERS_ADDRESS_SPECIAL, 0x42);
+    printf("* modbus_write_register (triggers invalid reply): ");
+    ASSERT_TRUE(rc == -1 && errno == EMBXILVAL, "");
+
     /** SLAVE REPLY **/
 
     printf("\nTEST SLAVE REPLY:\n");

tests/unit-test-server.c

@@ -210,8 +210,17 @@ int main(int argc, char *argv[])
                 // The valid length is lengths of header + checkum + FC + address + value
                 // (max 12)
                 rc = 34;
-                printf("Special modbus_write_bit detected. Inject a wrong rc value (%d) "
-                       "in modbus_reply\n",
+                printf(
+                    "Special modbus_write_bit detected. Inject a wrong length value (%d) "
+                    "in modbus_reply\n",
+                    rc);
+            }
+        } else if (query[header_length] == MODBUS_FC_WRITE_SINGLE_REGISTER) {
+            if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1) ==
+                UT_REGISTERS_ADDRESS_SPECIAL) {
+                rc = 45;
+                printf("Special modbus_write_register detected. Inject a wrong length "
+                       "value (%d) in modbus_reply\n",
                        rc);
             }
         }