fix!: Prevent API from accepting seed or public key for account (#4404)

The API would allow seeds (and public keys) to be used in place of
accounts at several locations in the API. For example, when calling
account_info, you could pass `"account": "foo"`. The string "foo" is
treated like a seed, so the method returns `actNotFound` (instead of
`actMalformed`, as most developers would expect). In the early days,
this was a convenience to make testing easier. However, it allows for
poor security practices, so it is no longer a good idea. Allowing a
secret or passphrase is now considered a bug. Previously, it was
controlled by the `strict` option on some methods. With this commit,
since the API does not interpret `account` as `seed`, the option
`strict` is no longer needed and is removed.

Removing this behavior from the API is a [breaking
change](https://xrpl.org/request-formatting.html#breaking-changes). One
could argue that it shouldn't be done without bumping the API version;
however, in this instance, there is no evidence that anyone is using the
API in the "legacy" way. Furthermore, it is a potential security hole,
as it allows users to send secrets to places where they are not needed,
where they could end up in logs, error messages, etc. There's no reason
to take such a risk with a seed/secret, since only the public address is
needed.

Resolves: #3329, #3330, #4337

BREAKING CHANGE: Remove non-strict account parsing (#3330)

Author: drlongle

src/ripple/app/main/Main.cpp

@@ -125,14 +125,12 @@ printHelp(const po::options_description& desc)
         << systemName() << "d [options] <command> <params>\n"
         << desc << std::endl
         << "Commands: \n"
-           "     account_currencies <account> [<ledger>] [strict]\n"
-           "     account_info <account>|<seed>|<pass_phrase>|<key> [<ledger>] "
-           "[strict]\n"
+           "     account_currencies <account> [<ledger>]\n"
+           "     account_info <account>|<key> [<ledger>]\n"
            "     account_lines <account> <account>|\"\" [<ledger>]\n"
            "     account_channels <account> <account>|\"\" [<ledger>]\n"
-           "     account_objects <account> [<ledger>] [strict]\n"
-           "     account_offers <account>|<account_public_key> [<ledger>] "
-           "[strict]\n"
+           "     account_objects <account> [<ledger>]\n"
+           "     account_offers <account>|<account_public_key> [<ledger>]\n"
            "     account_tx accountID [ledger_index_min [ledger_index_max "
            "[limit "
            "]]] [binary]\n"

src/ripple/net/impl/RPCCall.cpp

@@ -775,11 +775,9 @@ class RPCParser
         return jvRequest;
     }
 
-    // owner_info <account>|<account_public_key> [strict]
-    // owner_info <seed>|<pass_phrase>|<key> [<ledger>] [strict]
-    // account_info <account>|<account_public_key> [strict]
-    // account_info <seed>|<pass_phrase>|<key> [<ledger>] [strict]
-    // account_offers <account>|<account_public_key> [<ledger>] [strict]
+    // owner_info <account>
+    // account_info <account> [<ledger>]
+    // account_offers <account> [<ledger>]
     Json::Value
     parseAccountItems(Json::Value const& jvParams)
     {
@@ -895,10 +893,7 @@ class RPCParser
             // Parameters 0 and 1 are accounts
             if (i < 2)
             {
-                if (parseBase58<PublicKey>(
-                        TokenType::AccountPublic, strParam) ||
-                    parseBase58<AccountID>(strParam) ||
-                    parseGenericSeed(strParam))
+                if (parseBase58<AccountID>(strParam))
                 {
                     jvRequest[accFields[i]] = std::move(strParam);
                 }
@@ -924,26 +919,15 @@ class RPCParser
     {
         std::string strIdent = jvParams[0u].asString();
         unsigned int iCursor = jvParams.size();
-        bool bStrict = false;
 
-        if (iCursor >= 2 && jvParams[iCursor - 1] == jss::strict)
-        {
-            bStrict = true;
-            --iCursor;
-        }
-
-        if (!parseBase58<PublicKey>(TokenType::AccountPublic, strIdent) &&
-            !parseBase58<AccountID>(strIdent) && !parseGenericSeed(strIdent))
+        if (!parseBase58<AccountID>(strIdent))
             return rpcError(rpcACT_MALFORMED);
 
         // Get info on account.
         Json::Value jvRequest(Json::objectValue);
 
         jvRequest[jss::account] = strIdent;
 
-        if (bStrict)
-            jvRequest[jss::strict] = 1;
-
         if (iCursor == 2 && !jvParseLedger(jvRequest, jvParams[1u].asString()))
             return rpcError(rpcLGR_IDX_MALFORMED);
 

src/ripple/rpc/handlers/AccountChannels.cpp

@@ -58,7 +58,7 @@ addChannel(Json::Value& jsonLines, SLE const& line)
 }
 
 // {
-//   account: <account>|<account_public_key>
+//   account: <account>
 //   ledger_hash : <ledger>
 //   ledger_index : <ledger_index>
 //   limit: integer                 // optional
@@ -76,26 +76,25 @@ doAccountChannels(RPC::JsonContext& context)
     if (!ledger)
         return result;
 
-    std::string strIdent(params[jss::account].asString());
-    AccountID accountID;
-
-    if (auto const err = RPC::accountFromString(accountID, strIdent))
-        return err;
+    auto id = parseBase58<AccountID>(params[jss::account].asString());
+    if (!id)
+    {
+        return rpcError(rpcACT_MALFORMED);
+    }
+    AccountID const accountID{std::move(id.value())};
 
     if (!ledger->exists(keylet::account(accountID)))
         return rpcError(rpcACT_NOT_FOUND);
 
     std::string strDst;
     if (params.isMember(jss::destination_account))
         strDst = params[jss::destination_account].asString();
-    auto hasDst = !strDst.empty();
 
-    AccountID raDstAccount;
-    if (hasDst)
-    {
-        if (auto const err = RPC::accountFromString(raDstAccount, strDst))
-            return err;
-    }
+    auto const raDstAccount = [&]() -> std::optional<AccountID> {
+        return strDst.empty() ? std::nullopt : parseBase58<AccountID>(strDst);
+    }();
+    if (!strDst.empty() && !raDstAccount)
+        return rpcError(rpcACT_MALFORMED);
 
     unsigned int limit;
     if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context))
@@ -109,10 +108,9 @@ doAccountChannels(RPC::JsonContext& context)
     {
         std::vector<std::shared_ptr<SLE const>> items;
         AccountID const& accountID;
-        bool hasDst;
-        AccountID const& raDstAccount;
+        std::optional<AccountID> const& raDstAccount;
     };
-    VisitData visitData = {{}, accountID, hasDst, raDstAccount};
+    VisitData visitData = {{}, accountID, raDstAccount};
     visitData.items.reserve(limit);
     uint256 startAfter = beast::zero;
     std::uint64_t startHint = 0;
@@ -180,8 +178,8 @@ doAccountChannels(RPC::JsonContext& context)
 
                 if (count <= limit && sleCur->getType() == ltPAYCHAN &&
                     (*sleCur)[sfAccount] == accountID &&
-                    (!visitData.hasDst ||
-                     visitData.raDstAccount == (*sleCur)[sfDestination]))
+                    (!visitData.raDstAccount ||
+                     *visitData.raDstAccount == (*sleCur)[sfDestination]))
                 {
                     visitData.items.emplace_back(sleCur);
                 }

src/ripple/rpc/handlers/AccountCurrenciesHandler.cpp

@@ -46,13 +46,14 @@ doAccountCurrencies(RPC::JsonContext& context)
         params.isMember(jss::account) ? params[jss::account].asString()
                                       : params[jss::ident].asString());
 
-    bool const bStrict =
-        params.isMember(jss::strict) && params[jss::strict].asBool();
-
     // Get info on account.
-    AccountID accountID;  // out param
-    if (auto jvAccepted = RPC::accountFromString(accountID, strIdent, bStrict))
-        return jvAccepted;
+    auto id = parseBase58<AccountID>(strIdent);
+    if (!id)
+    {
+        RPC::inject_error(rpcACT_MALFORMED, result);
+        return result;
+    }
+    auto const accountID{std::move(id.value())};
 
     if (!ledger->exists(keylet::account(accountID)))
         return rpcError(rpcACT_NOT_FOUND);

src/ripple/rpc/handlers/AccountInfo.cpp

@@ -34,8 +34,6 @@ namespace ripple {
 
 // {
 //   account: <ident>,
-//   strict: <bool>        // optional (default false)
-//                         //   if true only allow public keys and addresses.
 //   ledger_hash : <ledger>
 //   ledger_index : <ledger_index>
 //   signer_lists : <bool> // optional (default false)
@@ -67,15 +65,14 @@ doAccountInfo(RPC::JsonContext& context)
     if (!ledger)
         return result;
 
-    bool bStrict = params.isMember(jss::strict) && params[jss::strict].asBool();
-    AccountID accountID;
-
     // Get info on account.
-
-    auto jvAccepted = RPC::accountFromString(accountID, strIdent, bStrict);
-
-    if (jvAccepted)
-        return jvAccepted;
+    auto id = parseBase58<AccountID>(strIdent);
+    if (!id)
+    {
+        RPC::inject_error(rpcACT_MALFORMED, result);
+        return result;
+    }
+    auto const accountID{std::move(id.value())};
 
     static constexpr std::
         array<std::pair<std::string_view, LedgerSpecificFlags>, 9>
@@ -113,6 +110,7 @@ doAccountInfo(RPC::JsonContext& context)
             return result;
         }
 
+        Json::Value jvAccepted(Json::objectValue);
         RPC::injectSLE(jvAccepted, *sleAccepted);
         result[jss::account_data] = jvAccepted;
 

src/ripple/rpc/handlers/AccountLines.cpp

@@ -30,17 +30,6 @@
 
 namespace ripple {
 
-struct VisitData
-{
-    std::vector<RPCTrustLine> items;
-    AccountID const& accountID;
-    bool hasPeer;
-    AccountID const& raPeerAccount;
-
-    bool ignoreDefault;
-    uint32_t foundCount;
-};
-
 void
 addLine(Json::Value& jsonLines, RPCTrustLine const& line)
 {
@@ -76,7 +65,7 @@ addLine(Json::Value& jsonLines, RPCTrustLine const& line)
 }
 
 // {
-//   account: <account>|<account_public_key>
+//   account: <account>
 //   ledger_hash : <ledger>
 //   ledger_index : <ledger_index>
 //   limit: integer                 // optional
@@ -96,33 +85,28 @@ doAccountLines(RPC::JsonContext& context)
     if (!ledger)
         return result;
 
-    std::string strIdent(params[jss::account].asString());
-    AccountID accountID;
-
-    if (auto jv = RPC::accountFromString(accountID, strIdent))
+    auto id = parseBase58<AccountID>(params[jss::account].asString());
+    if (!id)
     {
-        for (auto it = jv.begin(); it != jv.end(); ++it)
-            result[it.memberName()] = *it;
+        RPC::inject_error(rpcACT_MALFORMED, result);
         return result;
     }
+    auto const accountID{std::move(id.value())};
 
     if (!ledger->exists(keylet::account(accountID)))
         return rpcError(rpcACT_NOT_FOUND);
 
     std::string strPeer;
     if (params.isMember(jss::peer))
         strPeer = params[jss::peer].asString();
-    auto hasPeer = !strPeer.empty();
 
-    AccountID raPeerAccount;
-    if (hasPeer)
+    auto const raPeerAccount = [&]() -> std::optional<AccountID> {
+        return strPeer.empty() ? std::nullopt : parseBase58<AccountID>(strPeer);
+    }();
+    if (!strPeer.empty() && !raPeerAccount)
     {
-        if (auto jv = RPC::accountFromString(raPeerAccount, strPeer))
-        {
-            for (auto it = jv.begin(); it != jv.end(); ++it)
-                result[it.memberName()] = *it;
-            return result;
-        }
+        RPC::inject_error(rpcACT_MALFORMED, result);
+        return result;
     }
 
     unsigned int limit;
@@ -138,8 +122,15 @@ doAccountLines(RPC::JsonContext& context)
         params[jss::ignore_default].asBool();
 
     Json::Value& jsonLines(result[jss::lines] = Json::arrayValue);
-    VisitData visitData = {
-        {}, accountID, hasPeer, raPeerAccount, ignoreDefault, 0};
+    struct VisitData
+    {
+        std::vector<RPCTrustLine> items;
+        AccountID const& accountID;
+        std::optional<AccountID> const& raPeerAccount;
+        bool ignoreDefault;
+        uint32_t foundCount;
+    };
+    VisitData visitData = {{}, accountID, raPeerAccount, ignoreDefault, 0};
     uint256 startAfter = beast::zero;
     std::uint64_t startHint = 0;
 
@@ -227,8 +218,8 @@ doAccountLines(RPC::JsonContext& context)
                             RPCTrustLine::makeItem(visitData.accountID, sleCur);
 
                         if (line &&
-                            (!visitData.hasPeer ||
-                             visitData.raPeerAccount ==
+                            (!visitData.raPeerAccount ||
+                             *visitData.raPeerAccount ==
                                  line->getAccountIDPeer()))
                         {
                             visitData.items.emplace_back(*line);

src/ripple/rpc/handlers/AccountObjects.cpp

@@ -39,7 +39,7 @@ namespace ripple {
 
 /** General RPC command that can retrieve objects in the account root.
     {
-      account: <account>|<account_public_key>
+      account: <account>
       ledger_hash: <string> // optional
       ledger_index: <string | unsigned integer> // optional
       type: <string> // optional, defaults to all account objects types
@@ -60,17 +60,13 @@ doAccountNFTs(RPC::JsonContext& context)
     if (ledger == nullptr)
         return result;
 
-    AccountID accountID;
+    auto id = parseBase58<AccountID>(params[jss::account].asString());
+    if (!id)
     {
-        auto const strIdent = params[jss::account].asString();
-        if (auto jv = RPC::accountFromString(accountID, strIdent))
-        {
-            for (auto it = jv.begin(); it != jv.end(); ++it)
-                result[it.memberName()] = *it;
-
-            return result;
-        }
+        RPC::inject_error(rpcACT_MALFORMED, result);
+        return result;
     }
+    auto const accountID{std::move(id.value())};
 
     if (!ledger->exists(keylet::account(accountID)))
         return rpcError(rpcACT_NOT_FOUND);
@@ -177,17 +173,13 @@ doAccountObjects(RPC::JsonContext& context)
     if (ledger == nullptr)
         return result;
 
-    AccountID accountID;
+    auto const id = parseBase58<AccountID>(params[jss::account].asString());
+    if (!id)
     {
-        auto const strIdent = params[jss::account].asString();
-        if (auto jv = RPC::accountFromString(accountID, strIdent))
-        {
-            for (auto it = jv.begin(); it != jv.end(); ++it)
-                result[it.memberName()] = *it;
-
-            return result;
-        }
+        RPC::inject_error(rpcACT_MALFORMED, result);
+        return result;
     }
+    auto const accountID{std::move(id.value())};
 
     if (!ledger->exists(keylet::account(accountID)))
         return rpcError(rpcACT_NOT_FOUND);

src/ripple/rpc/handlers/AccountOffers.cpp

@@ -47,7 +47,7 @@ appendOfferJson(std::shared_ptr<SLE const> const& offer, Json::Value& offers)
 };
 
 // {
-//   account: <account>|<account_public_key>
+//   account: <account>
 //   ledger_hash : <ledger>
 //   ledger_index : <ledger_index>
 //   limit: integer                 // optional
@@ -65,16 +65,13 @@ doAccountOffers(RPC::JsonContext& context)
     if (!ledger)
         return result;
 
-    std::string strIdent(params[jss::account].asString());
-    AccountID accountID;
-
-    if (auto jv = RPC::accountFromString(accountID, strIdent))
+    auto id = parseBase58<AccountID>(params[jss::account].asString());
+    if (!id)
     {
-        for (auto it = jv.begin(); it != jv.end(); ++it)
-            result[it.memberName()] = (*it);
-
+        RPC::inject_error(rpcACT_MALFORMED, result);
         return result;
     }
+    auto const accountID{std::move(id.value())};
 
     // Get info on account.
     result[jss::account] = toBase58(accountID);