From 71f52d96b51bda2a2f00374237f368e980396692 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Fri, 19 Feb 2021 08:51:17 -0800 Subject: 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 Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0 --- redfish-core/lib/managers.hpp | 45 +++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) (limited to 'redfish-core/lib/managers.hpp') 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 "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 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 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) -- cgit v1.2.3