From d75a6edc58d69e187da6945257cae9a8afe795fd Mon Sep 17 00:00:00 2001
From: Scott Determan <scott.determan@yahoo.com>
Date: Thu, 2 Nov 2023 18:19:21 -0400
Subject: [PATCH] fix: check for valid public key in attestations (#4798)

---
 src/ripple/app/tx/impl/XChainBridge.cpp |  3 ++
 src/test/app/XChain_test.cpp            | 68 +++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/src/ripple/app/tx/impl/XChainBridge.cpp b/src/ripple/app/tx/impl/XChainBridge.cpp
index 3e1ae8719..934a88f55 100644
--- a/src/ripple/app/tx/impl/XChainBridge.cpp
+++ b/src/ripple/app/tx/impl/XChainBridge.cpp
@@ -1214,6 +1214,9 @@ attestationPreflight(PreflightContext const& ctx)
     if (ctx.tx.getFlags() & tfUniversalMask)
         return temINVALID_FLAG;
 
+    if (!publicKeyType(ctx.tx[sfPublicKey]))
+        return temMALFORMED;
+
     auto const att = toClaim<TAttestation>(ctx.tx);
     if (!att)
         return temMALFORMED;
diff --git a/src/test/app/XChain_test.cpp b/src/test/app/XChain_test.cpp
index 76e61ad4c..c96efc1b9 100644
--- a/src/test/app/XChain_test.cpp
+++ b/src/test/app/XChain_test.cpp
@@ -4210,6 +4210,73 @@ struct XChain_test : public beast::unit_test::suite,
         }
     }
 
+    void
+    testBadPublicKey()
+    {
+        using namespace jtx;
+
+        testcase("Bad attestations");
+        {
+            // Create a bridge and add an attestation with a bad public key
+            XEnv scEnv(*this, true);
+            std::uint32_t const claimID = 1;
+            std::optional<Account> dst{scBob};
+            auto const amt = XRP(1000);
+            scEnv.tx(create_bridge(Account::master, jvb))
+                .tx(jtx::signers(Account::master, quorum, signers))
+                .close();
+            scEnv.tx(xchain_create_claim_id(scAlice, jvb, reward, mcAlice))
+                .close();
+            auto jvAtt = claim_attestation(
+                scAttester,
+                jvb,
+                mcAlice,
+                amt,
+                payees[UT_XCHAIN_DEFAULT_QUORUM],
+                true,
+                claimID,
+                dst,
+                signers[UT_XCHAIN_DEFAULT_QUORUM]);
+            {
+                // Change to an invalid keytype
+                auto k = jvAtt["PublicKey"].asString();
+                k.at(1) = '9';
+                jvAtt["PublicKey"] = k;
+            }
+            scEnv.tx(jvAtt, ter(temMALFORMED)).close();
+        }
+        {
+            // Create a bridge and add an create account attestation with a bad
+            // public key
+            XEnv scEnv(*this, true);
+            std::uint32_t const createCount = 1;
+            Account dst{scBob};
+            auto const amt = XRP(1000);
+            auto const rewardAmt = XRP(1);
+            scEnv.tx(create_bridge(Account::master, jvb))
+                .tx(jtx::signers(Account::master, quorum, signers))
+                .close();
+            auto jvAtt = create_account_attestation(
+                scAttester,
+                jvb,
+                mcAlice,
+                amt,
+                rewardAmt,
+                payees[UT_XCHAIN_DEFAULT_QUORUM],
+                true,
+                createCount,
+                dst,
+                signers[UT_XCHAIN_DEFAULT_QUORUM]);
+            {
+                // Change to an invalid keytype
+                auto k = jvAtt["PublicKey"].asString();
+                k.at(1) = '9';
+                jvAtt["PublicKey"] = k;
+            }
+            scEnv.tx(jvAtt, ter(temMALFORMED)).close();
+        }
+    }
+
     void
     run() override
     {
@@ -4227,6 +4294,7 @@ struct XChain_test : public beast::unit_test::suite,
         testXChainCreateAccount();
         testFeeDipsIntoReserve();
         testXChainDeleteDoor();
+        testBadPublicKey();
     }
 };