commit 71f52d96b51bda2a2f00374237f368e980396692
author Ed Tanous <edtanous@google.com> Fri Feb 19 08:51:17 2021 -0800
committer Ed Tanous <ed@tanous.net> Fri Feb 19 20:39:57 2021 +0000
tree 1f14b4489ac5b383dbcadc055f7aa42a0e5c6dbc
parent 797ac9a28e0fc9d156a143aa84457360a8bb6fcb

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

include/dbus_monitor.hpp

diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp
index 18085a9..4337467 100644
--- a/include/dbus_monitor.hpp
+++ b/include/dbus_monitor.hpp
@@ -97,7 +97,8 @@
         return 0;
     }
 
-    connection->sendText(j.dump());
+    connection->sendText(
+        j.dump(2, ' ', true, nlohmann::json::error_handler_t::replace));
     return 0;
 }
 

include/openbmc_dbus_rest.hpp

diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 466ddc1..581e40b 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -519,7 +519,9 @@
                              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 @@
     {
         // 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