commit 2ff15bf656abaf9c29a05d782b2702f14528054a
author Snehalatha Venkatesh <snehalathax.v@intel.com> Thu Dec 30 11:51:14 2021 +0000
committer Snehalatha Venkatesh <snehalathax.v@intel.com> Mon Jan 17 19:16:02 2022 +0000
tree 74d3886ad36507021561ac8a3e3b3241b487dfb7
parent 0fac9bf4a060ab2b25133f329fa7602838b3dd3e

Add extra validation for GetPayloadData

Issue: GetPayloadData gives command response error when
greater data length is given.

Fix: Added extra validation to give correct response

Tested:
Before fix:
1. To get datalength
Command : ipmitool raw 0x30 0xd6 0x0 0x0
Response:
 00 00 00 cb 36 04 00 2a 5f 12 28 00 01 6d 2e 52
 5c
Case-1: When data length is invalid
Command : ipmitool raw 0x30 0xd6 1 0 0 0 0 0 0 0 0 0x05
Response:
Unable to send RAW command (channel=0x0 netfn=0x30 lun=0x0 cmd=0xd6
rsp=0xce): Command response could not be provided
Case-2:When data length is valid
Command : ipmitool raw 0x30 0xd6 1 0 0x09 0 0 0 0xcb 0 0 0
Response:
Unable to send RAW command (channel=0x0 netfn=0x30 lun=0x0 cmd=0xd6
rsp=0xce): Command response could not be provided

After fix:
1. Repeat 1 in before fix to get data length
Case-1: When data length is invalid
Command : ipmitool raw 0x30 0xd6 1 0 0 0 0 0 0 0 0 0x05
Response:
Unable to send RAW command (channel=0x0 netfn=0x30 lun=0x0 cmd=0xd6
rsp=0xcc): Invalid data field in request
Case-2:When data length is valid
Command : ipmitool raw 0x30 0xd6 1 0 0x09 0 0 0 0xcb 0 0 0
Response:     //Success

Signed-off-by: Snehalatha Venkatesh <snehalathax.v@intel.com>
Change-Id: I85d9648cb48991510c3a016a9d6bc6ef65904259

src/biosconfigcommands.cpp

diff --git a/src/biosconfigcommands.cpp b/src/biosconfigcommands.cpp
index 9d40f5d..3ae4582 100644
--- a/src/biosconfigcommands.cpp
+++ b/src/biosconfigcommands.cpp
@@ -1011,6 +1011,16 @@
                 std::string payloadFilePath =
                     "/var/oob/Payload" + std::to_string(payloadType);
 
+                if (length < static_cast<uint32_t>(maxGetPayloadDataSize))
+                {
+                    phosphor::logging::log<phosphor::logging::level::ERR>(
+                        "ipmiOEMGetPayload: length > maxGetPayloadDataSize",
+                        phosphor::logging::entry("LENGTH=%d", length),
+                        phosphor::logging::entry("maxGetPayloadDataSize=%d",
+                                                 maxGetPayloadDataSize));
+                    return ipmi::responseInvalidFieldRequest();
+                }
+
                 std::ifstream ifs(payloadFilePath, std::ios::in |
                                                        std::ios::binary |
                                                        std::ios::ate);
@@ -1026,7 +1036,15 @@
                 // Total file data within given offset
                 if (fileSize < static_cast<uint64_t>(offset))
                 {
-                    ifs.close();
+                    phosphor::logging::log<phosphor::logging::level::ERR>(
+                        "ipmiOEMGetPayload: filesize < offset");
+                    return ipmi::responseInvalidFieldRequest();
+                }
+
+                if ((static_cast<uint64_t>(fileSize) - offset) < length)
+                {
+                    phosphor::logging::log<phosphor::logging::level::ERR>(
+                        "ipmiOEMGetPayload: (filesize - offset) < length ");
                     return ipmi::responseInvalidFieldRequest();
                 }