commit 2332e0572300d6e490704b0d5fea720744594764
author Andrew Jeffery <andrew@codeconstruct.com.au> Tue Oct 08 13:52:34 2024 +1030
committer Andrew Jeffery <andrew@codeconstruct.com.au> Tue Oct 08 14:10:31 2024 +1030
tree cbdb98a36ab7b235c5b6e54fbfa7d7ad249e8970
parent 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47

Revert "dsp: platform: Fix decode_set_event_receiver_req()"

This reverts commit 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47.

As found in openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting
request size") the change in 8c43abb70aea ("dsp: platform: Fix
decode_set_event_receiver_req()") reduces the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES and causes an out-of-bounds access.

A new macro should be introduced rather than changing the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES.

Change-Id: I922c7eff86919f513e302bec393c0a516046e923
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

tests/dsp/platform.cpp

diff --git a/tests/dsp/platform.cpp b/tests/dsp/platform.cpp
index bd615c3..e937dec 100644
--- a/tests/dsp/platform.cpp
+++ b/tests/dsp/platform.cpp
@@ -3539,8 +3539,10 @@
     uint8_t eventReceiverAddressInfo = 0x08;
     uint16_t heartbeatTimer = 0x78;
 
-    PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-                                   PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+    std::vector<uint8_t> requestMsg(hdrSize +
+                                    PLDM_SET_EVENT_RECEIVER_REQ_BYTES);
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+    auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
 
     auto rc = encode_set_event_receiver_req(
         0, eventMessageGlobalEnable, transportProtocolType,
@@ -3564,8 +3566,10 @@
     uint8_t eventReceiverAddressInfo = 0x08;
     uint16_t heartbeatTimer = 0;
 
-    PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-                                   PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+    std::vector<uint8_t> requestMsg(hdrSize +
+                                    PLDM_SET_EVENT_RECEIVER_REQ_BYTES);
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+    auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
 
     auto rc = encode_set_event_receiver_req(
         0, eventMessageGlobalEnable, transportProtocolType,
@@ -3635,8 +3639,9 @@
 
 TEST(SetEventReceiver, testGoodDecodeRequest)
 {
-    PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-                                   PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+
+    std::array<uint8_t, hdrSize + PLDM_SET_EVENT_RECEIVER_REQ_BYTES>
+        requestMsg{};
 
     uint8_t eventMessageGlobalEnable =
         PLDM_EVENT_MESSAGE_GLOBAL_ENABLE_ASYNC_KEEP_ALIVE;
@@ -3644,6 +3649,8 @@
     uint8_t eventReceiverAddressInfo = 0x08;
     uint16_t heartbeatTimer = 0x78;
 
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+    auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
     struct pldm_set_event_receiver_req* req =
         // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         reinterpret_cast<struct pldm_set_event_receiver_req*>(request->payload);
@@ -3658,11 +3665,9 @@
     uint8_t reteventReceiverAddressInfo;
     uint16_t retheartbeatTimer;
     auto rc = decode_set_event_receiver_req(
-        request,
-        PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-            PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES,
-        &reteventMessageGlobalEnable, &rettransportProtocolType,
-        &reteventReceiverAddressInfo, &retheartbeatTimer);
+        request, requestMsg.size() - hdrSize, &reteventMessageGlobalEnable,
+        &rettransportProtocolType, &reteventReceiverAddressInfo,
+        &retheartbeatTimer);
 
     EXPECT_EQ(rc, PLDM_SUCCESS);
     EXPECT_EQ(eventMessageGlobalEnable, reteventMessageGlobalEnable);
@@ -3673,14 +3678,11 @@
 
 TEST(SetEventReceiver, testBadDecodeRequest)
 {
-    PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-                                   PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+    std::array<uint8_t, hdrSize + PLDM_SET_EVENT_RECEIVER_REQ_BYTES>
+        requestMsg{};
 
-    auto rc = decode_set_event_receiver_req(
-        NULL,
-        PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-            PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES,
-        NULL, NULL, NULL, NULL);
+    auto rc = decode_set_event_receiver_req(NULL, requestMsg.size() - hdrSize,
+                                            NULL, NULL, NULL, NULL);
     EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
 
     uint8_t eventMessageGlobalEnable =
@@ -3689,6 +3691,8 @@
     uint8_t eventReceiverAddressInfo = 0x08;
     uint16_t heartbeatTimer = 0x78;
 
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+    auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
     struct pldm_set_event_receiver_req* req =
         // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         reinterpret_cast<struct pldm_set_event_receiver_req*>(request->payload);
@@ -3702,24 +3706,11 @@
     uint8_t rettransportProtocolType;
     uint8_t reteventReceiverAddressInfo;
     uint16_t retheartbeatTimer;
-
-#ifdef NDEBUG
     rc = decode_set_event_receiver_req(
-        request,
-        PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-            PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES - 1,
-        &reteventMessageGlobalEnable, &rettransportProtocolType,
-        &reteventReceiverAddressInfo, &retheartbeatTimer);
+        request, requestMsg.size() - hdrSize - 1, &reteventMessageGlobalEnable,
+        &rettransportProtocolType, &reteventReceiverAddressInfo,
+        &retheartbeatTimer);
     EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
-#else
-    EXPECT_DEATH(decode_set_event_receiver_req(
-                     request,
-                     PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
-                         PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES - 1,
-                     &reteventMessageGlobalEnable, &rettransportProtocolType,
-                     &reteventReceiverAddressInfo, &retheartbeatTimer),
-                 "ctx->remaining >= 0");
-#endif
 }
 
 TEST(decodeNumericSensorPdrData, Uint8Test)