Fix nlohmann::json::dump calls

The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases.  http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly.  This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.

This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call.  For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.

Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases.  Observed correct responses.  I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0

1 files changed, 35 insertions, 10 deletions

diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index 8953d7a46b..ea0589e862 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -901,7 +901,10 @@ inline CreatePIDRet createPidInterface(
                 "PositiveHysteresis", doubles["PositiveHysteresis"],
                 "NegativeHysteresis", doubles["NegativeHysteresis"]))
         {
-            BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+            BMCWEB_LOG_ERROR
+                << "Illegal Property "
+                << it.value().dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace);
             return CreatePIDRet::fail;
         }
         if (zones)
@@ -1007,7 +1010,10 @@ inline CreatePIDRet createPidInterface(
                                           failSafePercent, "MinThermalOutput",
                                           minThermalOutput))
         {
-            BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+            BMCWEB_LOG_ERROR
+                << "Illegal Property "
+                << it.value().dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace);
             return CreatePIDRet::fail;
         }
 
@@ -1018,8 +1024,11 @@ inline CreatePIDRet createPidInterface(
             if (!redfish::json_util::readJson(*chassisContainer, response->res,
                                               "@odata.id", chassisId))
             {
-                BMCWEB_LOG_ERROR << "Illegal Property "
-                                 << chassisContainer->dump();
+                BMCWEB_LOG_ERROR
+                    << "Illegal Property "
+                    << chassisContainer->dump(
+                           2, ' ', true,
+                           nlohmann::json::error_handler_t::replace);
                 return CreatePIDRet::fail;
             }
 
@@ -1056,7 +1065,10 @@ inline CreatePIDRet createPidInterface(
                 "NegativeHysteresis", negativeHysteresis, "Direction",
                 direction))
         {
-            BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+            BMCWEB_LOG_ERROR
+                << "Illegal Property "
+                << it.value().dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace);
             return CreatePIDRet::fail;
         }
 
@@ -1089,8 +1101,11 @@ inline CreatePIDRet createPidInterface(
                 if (!redfish::json_util::readJson(step, response->res, "Target",
                                                   target, "Output", out))
                 {
-                    BMCWEB_LOG_ERROR << "Illegal Property "
-                                     << it.value().dump();
+                    BMCWEB_LOG_ERROR
+                        << "Illegal Property "
+                        << it.value().dump(
+                               2, ' ', true,
+                               nlohmann::json::error_handler_t::replace);
                     return CreatePIDRet::fail;
                 }
                 readings.emplace_back(target);
@@ -1331,7 +1346,10 @@ struct SetPIDValues : std::enable_shared_from_this<SetPIDValues>
                 "FanControllers", fanControllers, "FanZones", fanZones,
                 "StepwiseControllers", stepwiseControllers, "Profile", profile))
         {
-            BMCWEB_LOG_ERROR << "Illegal Property " << data.dump();
+            BMCWEB_LOG_ERROR
+                << "Illegal Property "
+                << data.dump(2, ' ', true,
+                             nlohmann::json::error_handler_t::replace);
             return;
         }
         configuration.emplace_back("PidControllers", std::move(pidControllers));
@@ -1996,7 +2014,10 @@ class Manager : public Node
             std::optional<nlohmann::json> openbmc;
             if (!redfish::json_util::readJson(*oem, res, "OpenBmc", openbmc))
             {
-                BMCWEB_LOG_ERROR << "Illegal Property " << oem->dump();
+                BMCWEB_LOG_ERROR
+                    << "Illegal Property "
+                    << oem->dump(2, ' ', true,
+                                 nlohmann::json::error_handler_t::replace);
                 return;
             }
             if (openbmc)
@@ -2004,7 +2025,11 @@ class Manager : public Node
                 std::optional<nlohmann::json> fan;
                 if (!redfish::json_util::readJson(*openbmc, res, "Fan", fan))
                 {
-                    BMCWEB_LOG_ERROR << "Illegal Property " << openbmc->dump();
+                    BMCWEB_LOG_ERROR
+                        << "Illegal Property "
+                        << openbmc->dump(
+                               2, ' ', true,
+                               nlohmann::json::error_handler_t::replace);
                     return;
                 }
                 if (fan)