Merge pull request #319 from wolfSSL/NVM-int-overflow

Fix integer overflow-prone offset+len bounds checks in NVM read handlers

Author: bigbrett

src/wh_nvm_flash.c

@@ -500,7 +500,8 @@ static int nfPartition_CheckDataRange(whNvmFlashContext* context,
         return WH_ERROR_BADARGS;
     }
 
-    if (byte_offset + byte_count > maxOffset) {
+    /* Use overflow-safe comparison */
+    if (byte_offset > maxOffset || byte_count > maxOffset - byte_offset) {
         return WH_ERROR_BADARGS;
     }
 
@@ -665,13 +666,14 @@ static int nfObject_ReadDataBytes(whNvmFlashContext* context, int partition,
     start = context->directory.objects[object_index].state.start;
     startOffset = nfPartition_DataOffset(context, partition) + start;
 
-    /* object bounds checks, do both to avoid integer overflow checks */
+    /* object bounds checks, use overflow-safe comparison */
     if (byte_offset >=
         (context->directory.objects[object_index].metadata.len)) {
         return WH_ERROR_BADARGS;
     }
-    if ((byte_offset + byte_count) >
-        (context->directory.objects[object_index].metadata.len)) {
+    if (byte_count >
+        (uint32_t)(context->directory.objects[object_index].metadata.len) -
+            byte_offset) {
         return WH_ERROR_BADARGS;
     }
 

src/wh_server_nvm.c

@@ -67,8 +67,8 @@ static int _HandleNvmRead(whServerContext* server, uint8_t* out_data,
     if (offset >= meta.len)
         return WH_ERROR_BADARGS;
 
-    /* Clamp length to object size */
-    if ((offset + len) > meta.len) {
+    /* Clamp length to object size, use overflow-safe comparison */
+    if (len > meta.len - offset) {
         len = meta.len - offset;
     }
 
@@ -427,8 +427,9 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
 
                 if (rc == 0) {
                     read_len = req.data_len;
-                    /* Clamp length to object size */
-                    if ((req.offset + read_len) > meta.len) {
+                    /* Clamp length to object size, use overflow-safe
+                     * comparison */
+                    if (read_len > meta.len - req.offset) {
                         read_len = meta.len - req.offset;
                     }
                 }

test/wh_test_clientserver.c

@@ -562,6 +562,29 @@ static int _testOutOfBoundsNvmReads(whClientContext* client,
         wh_Client_NvmReadResponse(client, &server_rc, &len, buffer));
     WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_BADARGS);
 
+    /* Test with large offset (UINT16_MAX), should fail since offset >=
+     * meta.len. Regression test for integer overflow safety in the
+     * offset+len check */
+    off = UINT16_MAX;
+    len = 1;
+    WH_TEST_RETURN_ON_FAIL(wh_Client_NvmReadRequest(client, id, off, len));
+    WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server));
+    WH_TEST_RETURN_ON_FAIL(
+        wh_Client_NvmReadResponse(client, &server_rc, &len, buffer));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_BADARGS);
+
+    /* Test clamping with offset at midpoint and len exceeding remaining object
+     * size. Verifies the overflow-safe comparison (len > meta.len - offset)
+     * correctly clamps when offset + len would exceed meta.len */
+    off = meta.len / 2;
+    len = meta.len;
+    WH_TEST_RETURN_ON_FAIL(wh_Client_NvmReadRequest(client, id, off, len));
+    WH_TEST_RETURN_ON_FAIL(wh_Server_HandleRequestMessage(server));
+    WH_TEST_RETURN_ON_FAIL(
+        wh_Client_NvmReadResponse(client, &server_rc, &len, buffer));
+    WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
+    WH_TEST_ASSERT_RETURN(len == meta.len - meta.len / 2);
+
     return WH_ERROR_OK;
 }