From 47d8899c9013a3b7fa384abaadf2e4bf9b6f274a Mon Sep 17 00:00:00 2001
From: Lee Clagett <code@leeclagett.com>
Date: Tue, 28 Nov 2023 18:05:15 -0500
Subject: [PATCH] Fix missing checks for IsObject in ZMQ jsonrpc reading

---
 src/rpc/daemon_messages.cpp             | 201 ++++++++++++++++++++++++
 tests/unit_tests/json_serialization.cpp |   7 +
 2 files changed, 208 insertions(+)

diff --git a/src/rpc/daemon_messages.cpp b/src/rpc/daemon_messages.cpp
index a7c921ff9..090a566c7 100644
--- a/src/rpc/daemon_messages.cpp
+++ b/src/rpc/daemon_messages.cpp
@@ -48,6 +48,11 @@ void GetHeight::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest) c
 
 void GetHeight::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, height, height);
 }
 
@@ -61,6 +66,11 @@ void GetBlocksFast::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void GetBlocksFast::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, block_ids, block_ids);
   GET_FROM_JSON_OBJECT(val, start_height, start_height);
   GET_FROM_JSON_OBJECT(val, prune, prune);
@@ -76,6 +86,11 @@ void GetBlocksFast::Response::doToJson(rapidjson::Writer<epee::byte_stream>& des
 
 void GetBlocksFast::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, blocks, blocks);
   GET_FROM_JSON_OBJECT(val, start_height, start_height);
   GET_FROM_JSON_OBJECT(val, current_height, current_height);
@@ -91,6 +106,11 @@ void GetHashesFast::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void GetHashesFast::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, known_hashes, known_hashes);
   GET_FROM_JSON_OBJECT(val, start_height, start_height);
 }
@@ -100,10 +120,16 @@ void GetHashesFast::Response::doToJson(rapidjson::Writer<epee::byte_stream>& des
   INSERT_INTO_JSON_OBJECT(dest, hashes, hashes);
   INSERT_INTO_JSON_OBJECT(dest, start_height, start_height);
   INSERT_INTO_JSON_OBJECT(dest, current_height, current_height);
+
 }
 
 void GetHashesFast::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, hashes, hashes);
   GET_FROM_JSON_OBJECT(val, start_height, start_height);
   GET_FROM_JSON_OBJECT(val, current_height, current_height);
@@ -117,6 +143,11 @@ void GetTransactions::Request::doToJson(rapidjson::Writer<epee::byte_stream>& de
 
 void GetTransactions::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, tx_hashes, tx_hashes);
 }
 
@@ -128,6 +159,11 @@ void GetTransactions::Response::doToJson(rapidjson::Writer<epee::byte_stream>& d
 
 void GetTransactions::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, txs, txs);
   GET_FROM_JSON_OBJECT(val, missed_hashes, missed_hashes);
 }
@@ -140,6 +176,11 @@ void KeyImagesSpent::Request::doToJson(rapidjson::Writer<epee::byte_stream>& des
 
 void KeyImagesSpent::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, key_images, key_images);
 }
 
@@ -150,6 +191,11 @@ void KeyImagesSpent::Response::doToJson(rapidjson::Writer<epee::byte_stream>& de
 
 void KeyImagesSpent::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, spent_status, spent_status);
 }
 
@@ -161,6 +207,11 @@ void GetTxGlobalOutputIndices::Request::doToJson(rapidjson::Writer<epee::byte_st
 
 void GetTxGlobalOutputIndices::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, tx_hash, tx_hash);
 }
 
@@ -171,6 +222,11 @@ void GetTxGlobalOutputIndices::Response::doToJson(rapidjson::Writer<epee::byte_s
 
 void GetTxGlobalOutputIndices::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, output_indices, output_indices);
 }
 
@@ -182,6 +238,11 @@ void SendRawTx::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest) co
 
 void SendRawTx::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, tx, tx);
   GET_FROM_JSON_OBJECT(val, relay, relay);
 }
@@ -194,6 +255,11 @@ void SendRawTx::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest) c
 
 void SendRawTx::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, relayed, relayed);
 }
 
@@ -205,6 +271,11 @@ void SendRawTxHex::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void SendRawTxHex::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, tx_as_hex, tx_as_hex);
   GET_FROM_JSON_OBJECT(val, relay, relay);
 }
@@ -219,6 +290,11 @@ void StartMining::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void StartMining::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, miner_address, miner_address);
   GET_FROM_JSON_OBJECT(val, threads_count, threads_count);
   GET_FROM_JSON_OBJECT(val, do_background_mining, do_background_mining);
@@ -266,6 +342,11 @@ void MiningStatus::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void MiningStatus::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, active, active);
   GET_FROM_JSON_OBJECT(val, speed, speed);
   GET_FROM_JSON_OBJECT(val, threads_count, threads_count);
@@ -288,6 +369,11 @@ void GetInfo::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest) con
 
 void GetInfo::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, info, info);
 }
 
@@ -314,6 +400,11 @@ void GetBlockHash::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void GetBlockHash::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, height, height);
 }
 
@@ -324,6 +415,11 @@ void GetBlockHash::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void GetBlockHash::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, hash, hash);
 }
 
@@ -342,6 +438,11 @@ void GetLastBlockHeader::Response::doToJson(rapidjson::Writer<epee::byte_stream>
 
 void GetLastBlockHeader::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, header, header);
 }
 
@@ -353,6 +454,11 @@ void GetBlockHeaderByHash::Request::doToJson(rapidjson::Writer<epee::byte_stream
 
 void GetBlockHeaderByHash::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, hash, hash);
 }
 
@@ -363,6 +469,11 @@ void GetBlockHeaderByHash::Response::doToJson(rapidjson::Writer<epee::byte_strea
 
 void GetBlockHeaderByHash::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, header, header);
 }
 
@@ -374,6 +485,11 @@ void GetBlockHeaderByHeight::Request::doToJson(rapidjson::Writer<epee::byte_stre
 
 void GetBlockHeaderByHeight::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, height, height);
 }
 
@@ -384,6 +500,11 @@ void GetBlockHeaderByHeight::Response::doToJson(rapidjson::Writer<epee::byte_str
 
 void GetBlockHeaderByHeight::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, header, header);
 }
 
@@ -395,6 +516,11 @@ void GetBlockHeadersByHeight::Request::doToJson(rapidjson::Writer<epee::byte_str
 
 void GetBlockHeadersByHeight::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, heights, heights);
 }
 
@@ -405,6 +531,11 @@ void GetBlockHeadersByHeight::Response::doToJson(rapidjson::Writer<epee::byte_st
 
 void GetBlockHeadersByHeight::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, headers, headers);
 }
 
@@ -424,6 +555,11 @@ void GetPeerList::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void GetPeerList::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, white_list, white_list);
   GET_FROM_JSON_OBJECT(val, gray_list, gray_list);
 }
@@ -436,6 +572,11 @@ void SetLogLevel::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void SetLogLevel::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, level, level);
 }
 
@@ -462,6 +603,11 @@ void GetTransactionPool::Response::doToJson(rapidjson::Writer<epee::byte_stream>
 
 void GetTransactionPool::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, transactions, transactions);
   GET_FROM_JSON_OBJECT(val, key_images, key_images);
 }
@@ -474,6 +620,11 @@ void HardForkInfo::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest)
 
 void HardForkInfo::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, version, version);
 }
 
@@ -484,6 +635,11 @@ void HardForkInfo::Response::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void HardForkInfo::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, info, info);
 }
 
@@ -499,6 +655,11 @@ void GetOutputHistogram::Request::doToJson(rapidjson::Writer<epee::byte_stream>&
 
 void GetOutputHistogram::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, amounts, amounts);
   GET_FROM_JSON_OBJECT(val, min_count, min_count);
   GET_FROM_JSON_OBJECT(val, max_count, max_count);
@@ -513,6 +674,11 @@ void GetOutputHistogram::Response::doToJson(rapidjson::Writer<epee::byte_stream>
 
 void GetOutputHistogram::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, histogram, histogram);
 }
 
@@ -524,6 +690,11 @@ void GetOutputKeys::Request::doToJson(rapidjson::Writer<epee::byte_stream>& dest
 
 void GetOutputKeys::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, outputs, outputs);
 }
 
@@ -534,6 +705,11 @@ void GetOutputKeys::Response::doToJson(rapidjson::Writer<epee::byte_stream>& des
 
 void GetOutputKeys::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, keys, keys);
 }
 
@@ -552,6 +728,11 @@ void GetRPCVersion::Response::doToJson(rapidjson::Writer<epee::byte_stream>& des
 
 void GetRPCVersion::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, version, version);
 }
 
@@ -562,6 +743,11 @@ void GetFeeEstimate::Request::doToJson(rapidjson::Writer<epee::byte_stream>& des
 
 void GetFeeEstimate::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, num_grace_blocks, num_grace_blocks);
 }
 
@@ -575,6 +761,11 @@ void GetFeeEstimate::Response::doToJson(rapidjson::Writer<epee::byte_stream>& de
 
 void GetFeeEstimate::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, estimated_base_fee, estimated_base_fee);
   GET_FROM_JSON_OBJECT(val, fee_mask, fee_mask);
   GET_FROM_JSON_OBJECT(val, size_scale, size_scale);
@@ -591,6 +782,11 @@ void GetOutputDistribution::Request::doToJson(rapidjson::Writer<epee::byte_strea
 
 void GetOutputDistribution::Request::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, amounts, amounts);
   GET_FROM_JSON_OBJECT(val, from_height, from_height);
   GET_FROM_JSON_OBJECT(val, to_height, to_height);
@@ -605,6 +801,11 @@ void GetOutputDistribution::Response::doToJson(rapidjson::Writer<epee::byte_stre
 
 void GetOutputDistribution::Response::fromJson(const rapidjson::Value& val)
 {
+  if (!val.IsObject())
+  {
+    throw json::WRONG_TYPE("json object");
+  }
+
   GET_FROM_JSON_OBJECT(val, status, status);
   GET_FROM_JSON_OBJECT(val, distributions, distributions);
 }
diff --git a/tests/unit_tests/json_serialization.cpp b/tests/unit_tests/json_serialization.cpp
index 9fa589139..aa46b68dc 100644
--- a/tests/unit_tests/json_serialization.cpp
+++ b/tests/unit_tests/json_serialization.cpp
@@ -13,6 +13,7 @@
 #include "cryptonote_basic/cryptonote_format_utils.h"
 #include "cryptonote_core/cryptonote_tx_utils.h"
 #include "serialization/json_object.h"
+#include "rpc/daemon_messages.h"
 
 
 namespace test
@@ -240,3 +241,9 @@ TEST(JsonSerialization, BulletproofTransaction)
     EXPECT_EQ(tx_bytes, tx_copy_bytes);
 }
 
+TEST(JsonRpcSerialization, HandlerFromJson)
+{
+  cryptonote::rpc::FullMessage req_full("{\"jsonrpc\":\"2.0\",\"method\":\"get_hashes_fast\",\"params\":[1]}", true);
+  cryptonote::rpc::GetHashesFast::Request request{};
+  EXPECT_THROW(request.fromJson(req_full.getMessage()), cryptonote::json::WRONG_TYPE);
+}