From 0054e4b64c86a1875228a8a04954d4d1f9ae6cb2 Mon Sep 17 00:00:00 2001
From: cyan317 <120398799+cindyyan317@users.noreply.github.com>
Date: Fri, 6 Sep 2024 10:37:08 +0100
Subject: [PATCH] fix: not forward admin API (#1629)

To merge to 2.2.3 branch.
It is different from the #1628 . I think this branch still forward
feature API to rippled.
---
 src/rpc/RPCEngine.hpp              |  8 +++++-
 src/rpc/RPCHelpers.cpp             | 17 +++++++++++++
 src/rpc/RPCHelpers.hpp             | 10 ++++++++
 tests/unit/rpc/RPCHelpersTests.cpp | 39 ++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/src/rpc/RPCEngine.hpp b/src/rpc/RPCEngine.hpp
index c728ea176..036e03001 100644
--- a/src/rpc/RPCEngine.hpp
+++ b/src/rpc/RPCEngine.hpp
@@ -22,6 +22,7 @@
 #include "data/BackendInterface.hpp"
 #include "rpc/Counters.hpp"
 #include "rpc/Errors.hpp"
+#include "rpc/RPCHelpers.hpp"
 #include "rpc/WorkQueue.hpp"
 #include "rpc/common/HandlerProvider.hpp"
 #include "rpc/common/Types.hpp"
@@ -134,8 +135,13 @@ class RPCEngine {
     Result
     buildResponse(web::Context const& ctx)
     {
-        if (forwardingProxy_.shouldForward(ctx))
+        if (forwardingProxy_.shouldForward(ctx)) {
+            // Disallow forwarding of the admin api, only user api is allowed for security reasons.
+            if (isAdminCmd(ctx.method, ctx.params))
+                return Result{Status{RippledError::rpcNO_PERMISSION}};
+
             return forwardingProxy_.forward(ctx);
+        }
 
         if (backend_->isTooBusy()) {
             LOG(log_.error()) << "Database is too busy. Rejecting request";
diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp
index b7f23ed81..2d10e4a3e 100644
--- a/src/rpc/RPCHelpers.cpp
+++ b/src/rpc/RPCHelpers.cpp
@@ -1273,6 +1273,23 @@ specifiesCurrentOrClosedLedger(boost::json::object const& request)
     return false;
 }
 
+bool
+isAdminCmd(std::string const& method, boost::json::object const& request)
+{
+    auto const isFieldSet = [&request](auto const field) {
+        return request.contains(field) and request.at(field).is_bool() and request.at(field).as_bool();
+    };
+
+    if (method == JS(ledger)) {
+        if (isFieldSet(JS(full)) or isFieldSet(JS(accounts)) or isFieldSet(JS(type)))
+            return true;
+    }
+
+    if (method == JS(feature) and request.contains(JS(vetoed)))
+        return true;
+    return false;
+}
+
 std::variant<ripple::uint256, Status>
 getNFTID(boost::json::object const& request)
 {
diff --git a/src/rpc/RPCHelpers.hpp b/src/rpc/RPCHelpers.hpp
index 23d7a306b..043762bd7 100644
--- a/src/rpc/RPCHelpers.hpp
+++ b/src/rpc/RPCHelpers.hpp
@@ -557,6 +557,16 @@ parseIssue(boost::json::object const& issue);
 bool
 specifiesCurrentOrClosedLedger(boost::json::object const& request);
 
+/**
+ * @brief Check whether a request requires administrative privileges on rippled side.
+ *
+ * @param method The method name to check
+ * @param request The request to check
+ * @return true if the request requires ADMIN role
+ */
+bool
+isAdminCmd(std::string const& method, boost::json::object const& request);
+
 /**
  * @brief Get the NFTID from the request
  *
diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp
index 1cb64efc6..e50c9af32 100644
--- a/tests/unit/rpc/RPCHelpersTests.cpp
+++ b/tests/unit/rpc/RPCHelpersTests.cpp
@@ -24,6 +24,7 @@
 #include "rpc/common/Types.hpp"
 #include "util/Fixtures.hpp"
 #include "util/MockPrometheus.hpp"
+#include "util/NameGenerator.hpp"
 #include "util/TestObject.hpp"
 
 #include <boost/asio/impl/spawn.hpp>
@@ -538,3 +539,41 @@ TEST_F(RPCHelpersTest, ParseIssue)
         std::runtime_error
     );
 }
+
+struct IsAdminCmdParamTestCaseBundle {
+    std::string testName;
+    std::string method;
+    std::string testJson;
+    bool expected;
+};
+
+struct IsAdminCmdParameterTest : public TestWithParam<IsAdminCmdParamTestCaseBundle> {};
+
+static auto
+generateTestValuesForParametersTest()
+{
+    return std::vector<IsAdminCmdParamTestCaseBundle>{
+        {"featureVetoedTrue", "feature", R"({"vetoed": true, "feature": "foo"})", true},
+        {"featureVetoedFalse", "feature", R"({"vetoed": false, "feature": "foo"})", true},
+        {"ledgerFullTrue", "ledger", R"({"full": true})", true},
+        {"ledgerAccountsTrue", "ledger", R"({"accounts": true})", true},
+        {"ledgerTypeTrue", "ledger", R"({"type": true})", true},
+        {"ledgerFullFalse", "ledger", R"({"full": false})", false},
+        {"ledgerAccountsFalse", "ledger", R"({"accounts": false})", false},
+        {"ledgerTypeFalse", "ledger", R"({"type": false})", false},
+        {"ledgerEntry", "ledger_entry", R"({"type": false})", false}
+    };
+}
+
+INSTANTIATE_TEST_CASE_P(
+    IsAdminCmdTest,
+    IsAdminCmdParameterTest,
+    ValuesIn(generateTestValuesForParametersTest()),
+    tests::util::NameGenerator
+);
+
+TEST_P(IsAdminCmdParameterTest, Test)
+{
+    auto const testBundle = GetParam();
+    EXPECT_EQ(isAdminCmd(testBundle.method, boost::json::parse(testBundle.testJson).as_object()), testBundle.expected);
+}