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
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 3db9f0c..148c703 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -443,7 +443,8 @@
{"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 @@
{"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 @@
{"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 @@
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 @@
{"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 @@
{"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 b794af7..1eeb65a 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -207,12 +207,20 @@
{
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 @@
{
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 @@
{
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 3b6062a..2703008 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -239,7 +239,9 @@
{
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 159eda2..4dfef0f 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -1403,8 +1403,11 @@
{
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 @@
"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 @@
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 @@
{
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 @@
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 @@
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 f7b76d1..be6f04d 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -434,7 +434,9 @@
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 290bac7..10b16f9 100644
--- a/redfish-core/lib/hypervisor_system.hpp
+++ b/redfish-core/lib/hypervisor_system.hpp
@@ -553,8 +553,11 @@
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 8953d7a..ea0589e 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -901,7 +901,10 @@
"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 @@
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 @@
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 @@
"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 @@
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 @@
"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 @@
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 @@
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 b6ca010..dbc6278 100644
--- a/redfish-core/lib/task.hpp
+++ b/redfish-core/lib/task.hpp
@@ -446,7 +446,9 @@
{"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;
}