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

11 files changed, 120 insertions, 38 deletions

diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 39ac39e625..e5cd74f906 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -438,7 +438,8 @@ class Connection :
             else
             {
                 res.jsonMode();
-                res.body() = res.jsonValue.dump(2, ' ', true);
+                res.body() = res.jsonValue.dump(
+                    2, ' ', true, nlohmann::json::error_handler_t::replace);
             }
         }
 
diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp
index 18085a930e..4337467bcc 100644
--- a/include/dbus_monitor.hpp
+++ b/include/dbus_monitor.hpp
@@ -97,7 +97,8 @@ inline int onPropertyUpdate(sd_bus_message* m, void* userdata,
         return 0;
     }
 
-    connection->sendText(j.dump());
+    connection->sendText(
+        j.dump(2, ' ', true, nlohmann::json::error_handler_t::replace));
     return 0;
 }
 
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 466ddc1d0a..581e40b31a 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -519,7 +519,9 @@ inline int convertJsonToDbus(sd_bus_message* m, const std::string& argType,
                              const nlohmann::json& inputJson)
 {
     int r = 0;
-    BMCWEB_LOG_DEBUG << "Converting " << inputJson.dump()
+    BMCWEB_LOG_DEBUG << "Converting "
+                     << inputJson.dump(2, ' ', true,
+                                       nlohmann::json::error_handler_t::replace)
                      << " to type: " << argType;
     const std::vector<std::string> argTypes = dbusArgSplit(argType);
 
@@ -914,7 +916,7 @@ inline int readDictEntryFromMessage(const std::string& typeCode,
     {
         // json doesn't support non-string keys.  If we hit this condition,
         // convert the result to a string so we can proceed
-        key = key.dump();
+        key = key.dump(2, ' ', true, nlohmann::json::error_handler_t::replace);
         keyPtr = key.get_ptr<const std::string*>();
         // in theory this can't fail now, but lets be paranoid about it
         // anyway
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 3db9f0ca79..148c703c9b 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -443,7 +443,8 @@ class Subscription
                               {"Name", "Event Log"},
                               {"Events", logEntryArray}};
 
-        this->sendEvent(msg.dump());
+        this->sendEvent(
+            msg.dump(2, ' ', true, nlohmann::json::error_handler_t::replace));
     }
 
 #ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES
@@ -506,7 +507,8 @@ class Subscription
                               {"Name", "Event Log"},
                               {"Events", logEntryArray}};
 
-        this->sendEvent(msg.dump());
+        this->sendEvent(
+            msg.dump(2, ' ', true, nlohmann::json::error_handler_t::replace));
     }
 #endif
 
@@ -551,7 +553,8 @@ class Subscription
             {"MetricReportDefinition", {{"@odata.id", metricReportDef}}},
             {"MetricValues", metricValuesArray}};
 
-        this->sendEvent(msg.dump());
+        this->sendEvent(
+            msg.dump(2, ' ', true, nlohmann::json::error_handler_t::replace));
     }
 
     void updateRetryConfig(const uint32_t retryAttempts,
@@ -813,7 +816,8 @@ class EventServiceManager
 
         const std::string tmpFile(std::string(eventServiceFile) + "_tmp");
         std::ofstream ofs(tmpFile, std::ios::out);
-        const auto& writeData = jsonData.dump();
+        const auto& writeData = jsonData.dump(
+            2, ' ', true, nlohmann::json::error_handler_t::replace);
         ofs << writeData;
         ofs.close();
 
@@ -1086,7 +1090,8 @@ class EventServiceManager
                     {"Name", "Event Log"},
                     {"Id", eventId},
                     {"Events", eventRecord}};
-                entry->sendEvent(msgJson.dump());
+                entry->sendEvent(msgJson.dump(
+                    2, ' ', true, nlohmann::json::error_handler_t::replace));
                 eventId++; // increament the eventId
             }
             else
@@ -1105,7 +1110,8 @@ class EventServiceManager
                 {"OriginOfCondition", "/ibm/v1/HMC/BroadcastService"},
                 {"Name", "Broadcast Message"},
                 {"Message", broadcastMsg}};
-            entry->sendEvent(msgJson.dump());
+            entry->sendEvent(msgJson.dump(
+                2, ' ', true, nlohmann::json::error_handler_t::replace));
         }
     }
 
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index b794af7fa1..1eeb65ad00 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -207,12 +207,20 @@ bool unpackValue(nlohmann::json& jsonValue, const std::string& key,
     {
         if (!jsonValue.is_array())
         {
-            messages::propertyValueTypeError(res, res.jsonValue.dump(), key);
+            messages::propertyValueTypeError(
+                res,
+                res.jsonValue.dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace),
+                key);
             return false;
         }
         if (jsonValue.size() != value.size())
         {
-            messages::propertyValueTypeError(res, res.jsonValue.dump(), key);
+            messages::propertyValueTypeError(
+                res,
+                res.jsonValue.dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace),
+                key);
             return false;
         }
         size_t index = 0;
@@ -227,7 +235,11 @@ bool unpackValue(nlohmann::json& jsonValue, const std::string& key,
     {
         if (!jsonValue.is_array())
         {
-            messages::propertyValueTypeError(res, res.jsonValue.dump(), key);
+            messages::propertyValueTypeError(
+                res,
+                res.jsonValue.dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace),
+                key);
             return false;
         }
 
@@ -246,11 +258,19 @@ bool unpackValue(nlohmann::json& jsonValue, const std::string& key,
         {
             if (ec == UnpackErrorCode::invalidType)
             {
-                messages::propertyValueTypeError(res, jsonValue.dump(), key);
+                messages::propertyValueTypeError(
+                    res,
+                    jsonValue.dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace),
+                    key);
             }
             else if (ec == UnpackErrorCode::outOfRange)
             {
-                messages::propertyValueNotInList(res, jsonValue.dump(), key);
+                messages::propertyValueNotInList(
+                    res,
+                    jsonValue.dump(2, ' ', true,
+                                   nlohmann::json::error_handler_t::replace),
+                    key);
             }
             return false;
         }
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 3b6062ab21..2703008923 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -239,7 +239,9 @@ inline void handleRoleMapPatch(
             {
                 BMCWEB_LOG_ERROR << "Can't delete the object";
                 messages::propertyValueTypeError(
-                    asyncResp->res, thisJson.dump(),
+                    asyncResp->res,
+                    thisJson.dump(2, ' ', true,
+                                  nlohmann::json::error_handler_t::replace),
                     "RemoteRoleMapping/" + std::to_string(index));
                 return;
             }
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 159eda2433..4dfef0fcfa 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -1403,8 +1403,11 @@ class EthernetInterface : public Node
     {
         if ((!input.is_array()) || input.empty())
         {
-            messages::propertyValueTypeError(asyncResp->res, input.dump(),
-                                             "IPv4StaticAddresses");
+            messages::propertyValueTypeError(
+                asyncResp->res,
+                input.dump(2, ' ', true,
+                           nlohmann::json::error_handler_t::replace),
+                "IPv4StaticAddresses");
             return;
         }
 
@@ -1432,7 +1435,10 @@ class EthernetInterface : public Node
                                          "Gateway", gateway))
                 {
                     messages::propertyValueFormatError(
-                        asyncResp->res, thisJson.dump(), pathString);
+                        asyncResp->res,
+                        thisJson.dump(2, ' ', true,
+                                      nlohmann::json::error_handler_t::replace),
+                        pathString);
                     return;
                 }
 
@@ -1552,7 +1558,10 @@ class EthernetInterface : public Node
                         return;
                     }
                     messages::propertyValueFormatError(
-                        asyncResp->res, thisJson.dump(), pathString);
+                        asyncResp->res,
+                        thisJson.dump(2, ' ', true,
+                                      nlohmann::json::error_handler_t::replace),
+                        pathString);
                     return;
                 }
 
@@ -1598,8 +1607,11 @@ class EthernetInterface : public Node
     {
         if (!input.is_array() || input.empty())
         {
-            messages::propertyValueTypeError(asyncResp->res, input.dump(),
-                                             "IPv6StaticAddresses");
+            messages::propertyValueTypeError(
+                asyncResp->res,
+                input.dump(2, ' ', true,
+                           nlohmann::json::error_handler_t::replace),
+                "IPv6StaticAddresses");
             return;
         }
         size_t entryIdx = 1;
@@ -1620,7 +1632,10 @@ class EthernetInterface : public Node
                                          prefixLength))
                 {
                     messages::propertyValueFormatError(
-                        asyncResp->res, thisJson.dump(), pathString);
+                        asyncResp->res,
+                        thisJson.dump(2, ' ', true,
+                                      nlohmann::json::error_handler_t::replace),
+                        pathString);
                     return;
                 }
 
@@ -1687,7 +1702,10 @@ class EthernetInterface : public Node
                         return;
                     }
                     messages::propertyValueFormatError(
-                        asyncResp->res, thisJson.dump(), pathString);
+                        asyncResp->res,
+                        thisJson.dump(2, ' ', true,
+                                      nlohmann::json::error_handler_t::replace),
+                        pathString);
                     return;
                 }
 
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index f7b76d1b22..be6f04df2f 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -434,7 +434,9 @@ class EventDestinationCollection : public Node
                 else
                 {
                     messages::propertyValueFormatError(
-                        asyncResp->res, mrdObj.dump(),
+                        asyncResp->res,
+                        mrdObj.dump(2, ' ', true,
+                                    nlohmann::json::error_handler_t::replace),
                         "MetricReportDefinitions");
                     return;
                 }
diff --git a/redfish-core/lib/hypervisor_system.hpp b/redfish-core/lib/hypervisor_system.hpp
index 290bac7188..10b16f9656 100644
--- a/redfish-core/lib/hypervisor_system.hpp
+++ b/redfish-core/lib/hypervisor_system.hpp
@@ -553,8 +553,11 @@ class HypervisorInterface : public Node
                                      address, "SubnetMask", subnetMask,
                                      "Gateway", gateway))
             {
-                messages::propertyValueFormatError(asyncResp->res,
-                                                   thisJson.dump(), pathString);
+                messages::propertyValueFormatError(
+                    asyncResp->res,
+                    thisJson.dump(2, ' ', true,
+                                  nlohmann::json::error_handler_t::replace),
+                    pathString);
                 return;
             }
 
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)
diff --git a/redfish-core/lib/task.hpp b/redfish-core/lib/task.hpp
index b6ca010ba4..dbc6278897 100644
--- a/redfish-core/lib/task.hpp
+++ b/redfish-core/lib/task.hpp
@@ -446,7 +446,9 @@ class Task : public Node
                 {"TargetUri", p.targetUri},
                 {"HttpOperation", p.httpOperation},
                 {"HttpHeaders", p.httpHeaders},
-                {"JsonBody", p.jsonBody.dump()}};
+                {"JsonBody",
+                 p.jsonBody.dump(2, ' ', true,
+                                 nlohmann::json::error_handler_t::replace)}};
         }
         asyncResp->res.jsonValue["PercentComplete"] = ptr->percentComplete;
     }