From dbe7c5c010bef2366e67a61e1f52dff0b75f15fe Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 16 Aug 2023 17:23:42 -0400 Subject: [PATCH] Replace all the per-field checks with a try/catch --- src/ripple/protocol/impl/Issue.cpp | 16 +- src/ripple/rpc/handlers/LedgerEntry.cpp | 600 +++++++++++------------- 2 files changed, 271 insertions(+), 345 deletions(-) diff --git a/src/ripple/protocol/impl/Issue.cpp b/src/ripple/protocol/impl/Issue.cpp index 23c5427419a..6d0be069a8a 100644 --- a/src/ripple/protocol/impl/Issue.cpp +++ b/src/ripple/protocol/impl/Issue.cpp @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -78,7 +79,7 @@ issueFromJson(Json::Value const& v) { if (!v.isObject()) { - Throw( + Throw( "issueFromJson can only be specified with a 'object' Json value"); } @@ -87,37 +88,34 @@ issueFromJson(Json::Value const& v) if (!curStr.isString()) { - Throw( + Throw( "issueFromJson currency must be a string Json value"); } auto const currency = to_currency(curStr.asString()); if (currency == badCurrency() || currency == noCurrency()) { - Throw( - "issueFromJson currency must be a valid currency"); + Throw("issueFromJson currency must be a valid currency"); } if (isXRP(currency)) { if (!issStr.isNull()) { - Throw("Issue, XRP should not have issuer"); + Throw("Issue, XRP should not have issuer"); } return xrpIssue(); } if (!issStr.isString()) { - Throw( - "issueFromJson issuer must be a string Json value"); + Throw("issueFromJson issuer must be a string Json value"); } auto const issuer = parseBase58(issStr.asString()); if (!issuer) { - Throw( - "issueFromJson issuer must be a valid account"); + Throw("issueFromJson issuer must be a valid account"); } return Issue{currency, *issuer}; diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index c3e4025ebb4..1f966f8d032 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -49,30 +50,17 @@ doLedgerEntry(RPC::JsonContext& context) bool bNodeBinary = false; LedgerEntryType expectedType = ltANY; - if (context.params.isMember(jss::index)) + try { - // XRPL Doc states "index" field is a string. check for this apiVersion - // 2 onwards. invalidParam error is thrown if the above condition is - // false - if (context.apiVersion > 1u && !context.params[jss::index].isString()) + if (context.params.isMember(jss::index)) { - uNodeIndex = beast::zero; - jvResult[jss::error] = "invalidParams"; - } - else if (!uNodeIndex.parseHex(context.params[jss::index].asString())) - { - uNodeIndex = beast::zero; - jvResult[jss::error] = "malformedRequest"; - } - } - else if (context.params.isMember(jss::account_root)) - { - if (context.apiVersion > 1u && - !context.params[jss::account_root].isString()) - { - jvResult[jss::error] = "invalidParams"; + if (!uNodeIndex.parseHex(context.params[jss::index].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else + else if (context.params.isMember(jss::account_root)) { expectedType = ltACCOUNT_ROOT; auto const account = parseBase58( @@ -82,136 +70,109 @@ doLedgerEntry(RPC::JsonContext& context) else uNodeIndex = keylet::account(*account).key; } - } - else if (context.params.isMember(jss::check)) - { - expectedType = ltCHECK; - // XRPL Doc states "check" field is a string. Check for this apiVersion - // 2 onwards. invalidParam error is thrown if the above condition is - // false - if (context.apiVersion > 1u && !context.params[jss::check].isString()) - { - uNodeIndex = beast::zero; - jvResult[jss::error] = "invalidParams"; - } - else if (!uNodeIndex.parseHex(context.params[jss::check].asString())) + else if (context.params.isMember(jss::check)) { - uNodeIndex = beast::zero; - jvResult[jss::error] = "malformedRequest"; - } - } - else if (context.params.isMember(jss::deposit_preauth)) - { - expectedType = ltDEPOSIT_PREAUTH; - - if (!context.params[jss::deposit_preauth].isObject()) - { - if (!context.params[jss::deposit_preauth].isString() || - !uNodeIndex.parseHex( - context.params[jss::deposit_preauth].asString())) + expectedType = ltCHECK; + if (!uNodeIndex.parseHex(context.params[jss::check].asString())) { uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } } - else if ( - !context.params[jss::deposit_preauth].isMember(jss::owner) || - !context.params[jss::deposit_preauth][jss::owner].isString() || - !context.params[jss::deposit_preauth].isMember(jss::authorized) || - !context.params[jss::deposit_preauth][jss::authorized].isString()) - { - jvResult[jss::error] = "malformedRequest"; - } - else + else if (context.params.isMember(jss::deposit_preauth)) { - auto const owner = parseBase58( - context.params[jss::deposit_preauth][jss::owner].asString()); + expectedType = ltDEPOSIT_PREAUTH; - auto const authorized = parseBase58( - context.params[jss::deposit_preauth][jss::authorized] - .asString()); - - if (!owner) - jvResult[jss::error] = "malformedOwner"; - else if (!authorized) - jvResult[jss::error] = "malformedAuthorized"; - else - uNodeIndex = keylet::depositPreauth(*owner, *authorized).key; - } - } - else if (context.params.isMember(jss::directory)) - { - expectedType = ltDIR_NODE; - if (context.params[jss::directory].isNull()) - { - jvResult[jss::error] = "malformedRequest"; - } - else if (!context.params[jss::directory].isObject()) - { - // XRPL Doc states "directory" field is a string. check for this - // apiVersion 2 onwards. invalidParam error is thrown if the above - // condition is false - if (context.apiVersion > 1u && - !context.params[jss::directory].isString()) + if (!context.params[jss::deposit_preauth].isObject()) { - uNodeIndex = beast::zero; - jvResult[jss::error] = "invalidParams"; + if (!context.params[jss::deposit_preauth].isString() || + !uNodeIndex.parseHex( + context.params[jss::deposit_preauth].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else if (!uNodeIndex.parseHex( - context.params[jss::directory].asString())) + else if ( + !context.params[jss::deposit_preauth].isMember(jss::owner) || + !context.params[jss::deposit_preauth][jss::owner].isString() || + !context.params[jss::deposit_preauth].isMember( + jss::authorized) || + !context.params[jss::deposit_preauth][jss::authorized] + .isString()) { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } + else + { + auto const owner = parseBase58( + context.params[jss::deposit_preauth][jss::owner] + .asString()); + + auto const authorized = parseBase58( + context.params[jss::deposit_preauth][jss::authorized] + .asString()); + + if (!owner) + jvResult[jss::error] = "malformedOwner"; + else if (!authorized) + jvResult[jss::error] = "malformedAuthorized"; + else + uNodeIndex = + keylet::depositPreauth(*owner, *authorized).key; + } } - else if ( - context.params[jss::directory].isMember(jss::sub_index) && - !context.params[jss::directory][jss::sub_index].isIntegral()) - { - jvResult[jss::error] = "malformedRequest"; - } - else + else if (context.params.isMember(jss::directory)) { - std::uint64_t uSubIndex = - context.params[jss::directory].isMember(jss::sub_index) - ? context.params[jss::directory][jss::sub_index].asUInt() - : 0; - - if (context.params[jss::directory].isMember(jss::dir_root)) + expectedType = ltDIR_NODE; + if (context.params[jss::directory].isNull()) { - uint256 uDirRoot; - - if (context.params[jss::directory].isMember(jss::owner)) - { - // May not specify both dir_root and owner. - jvResult[jss::error] = "malformedRequest"; - } - else if ( - context.apiVersion > 1u && - !context.params[jss::directory][jss::dir_root].isString()) - { - jvResult[jss::error] = "invalidParams"; - } - else if (!uDirRoot.parseHex( - context.params[jss::directory][jss::dir_root] - .asString())) + jvResult[jss::error] = "malformedRequest"; + } + else if (!context.params[jss::directory].isObject()) + { + if (!uNodeIndex.parseHex( + context.params[jss::directory].asString())) { uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } - else - { - uNodeIndex = keylet::page(uDirRoot, uSubIndex).key; - } } - else if (context.params[jss::directory].isMember(jss::owner)) + else if ( + context.params[jss::directory].isMember(jss::sub_index) && + !context.params[jss::directory][jss::sub_index].isIntegral()) { - if (context.apiVersion > 1u && - !context.params[jss::directory][jss::owner].isString()) + jvResult[jss::error] = "malformedRequest"; + } + else + { + std::uint64_t uSubIndex = + context.params[jss::directory].isMember(jss::sub_index) + ? context.params[jss::directory][jss::sub_index].asUInt() + : 0; + + if (context.params[jss::directory].isMember(jss::dir_root)) { - jvResult[jss::error] = "invalidParams"; + uint256 uDirRoot; + + if (context.params[jss::directory].isMember(jss::owner)) + { + // May not specify both dir_root and owner. + jvResult[jss::error] = "malformedRequest"; + } + else if (!uDirRoot.parseHex( + context.params[jss::directory][jss::dir_root] + .asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } + else + { + uNodeIndex = keylet::page(uDirRoot, uSubIndex).key; + } } - else + else if (context.params[jss::directory].isMember(jss::owner)) { auto const ownerID = parseBase58( context.params[jss::directory][jss::owner].asString()); @@ -227,275 +188,242 @@ doLedgerEntry(RPC::JsonContext& context) .key; } } - } - else - { - jvResult[jss::error] = "malformedRequest"; + else + { + jvResult[jss::error] = "malformedRequest"; + } } } - } - else if (context.params.isMember(jss::escrow)) - { - expectedType = ltESCROW; - if (!context.params[jss::escrow].isObject()) + else if (context.params.isMember(jss::escrow)) { - if (context.apiVersion > 1u && - !context.params[jss::escrow].isString()) + expectedType = ltESCROW; + if (!context.params[jss::escrow].isObject()) { - jvResult[jss::error] = "invalidParams"; + if (!uNodeIndex.parseHex( + context.params[jss::escrow].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else if (!uNodeIndex.parseHex( - context.params[jss::escrow].asString())) + else if ( + !context.params[jss::escrow].isMember(jss::owner) || + !context.params[jss::escrow].isMember(jss::seq) || + !context.params[jss::escrow][jss::seq].isIntegral()) { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } - } - else if ( - !context.params[jss::escrow].isMember(jss::owner) || - !context.params[jss::escrow].isMember(jss::seq) || - !context.params[jss::escrow][jss::seq].isIntegral()) - { - jvResult[jss::error] = "malformedRequest"; - } - else if ( - context.apiVersion > 1u && - !context.params[jss::escrow][jss::owner].isString()) - { - jvResult[jss::error] = "invalidParams"; - } - else - { - auto const id = parseBase58( - context.params[jss::escrow][jss::owner].asString()); - if (!id) - jvResult[jss::error] = "malformedOwner"; else - uNodeIndex = - keylet::escrow( - *id, context.params[jss::escrow][jss::seq].asUInt()) - .key; + { + auto const id = parseBase58( + context.params[jss::escrow][jss::owner].asString()); + if (!id) + jvResult[jss::error] = "malformedOwner"; + else + uNodeIndex = + keylet::escrow( + *id, context.params[jss::escrow][jss::seq].asUInt()) + .key; + } } - } - else if (context.params.isMember(jss::offer)) - { - expectedType = ltOFFER; - if (!context.params[jss::offer].isObject()) + else if (context.params.isMember(jss::offer)) { - if (context.apiVersion > 1u && - !context.params[jss::offer].isString()) + expectedType = ltOFFER; + if (!context.params[jss::offer].isObject()) { - jvResult[jss::error] = "invalidParams"; + if (!uNodeIndex.parseHex(context.params[jss::offer].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else if (!uNodeIndex.parseHex( - context.params[jss::offer].asString())) + else if ( + !context.params[jss::offer].isMember(jss::account) || + !context.params[jss::offer].isMember(jss::seq) || + !context.params[jss::offer][jss::seq].isIntegral()) { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } - } - else if ( - !context.params[jss::offer].isMember(jss::account) || - !context.params[jss::offer].isMember(jss::seq) || - !context.params[jss::offer][jss::seq].isIntegral()) - { - jvResult[jss::error] = "malformedRequest"; - } - else if ( - context.apiVersion > 1u && - !context.params[jss::offer][jss::account].isString()) - { - jvResult[jss::error] = "invalidParams"; - } - else - { - auto const id = parseBase58( - context.params[jss::offer][jss::account].asString()); - if (!id) - jvResult[jss::error] = "malformedAddress"; else - uNodeIndex = - keylet::offer( - *id, context.params[jss::offer][jss::seq].asUInt()) - .key; - } - } - else if (context.params.isMember(jss::payment_channel)) - { - expectedType = ltPAYCHAN; - - if (context.apiVersion > 1u && - !context.params[jss::payment_channel].isString()) - { - jvResult[jss::error] = "invalidParams"; + { + auto const id = parseBase58( + context.params[jss::offer][jss::account].asString()); + if (!id) + jvResult[jss::error] = "malformedAddress"; + else + uNodeIndex = + keylet::offer( + *id, context.params[jss::offer][jss::seq].asUInt()) + .key; + } } - else if (!uNodeIndex.parseHex( - context.params[jss::payment_channel].asString())) + else if (context.params.isMember(jss::payment_channel)) { - uNodeIndex = beast::zero; - jvResult[jss::error] = "malformedRequest"; - } - } - else if (context.params.isMember(jss::ripple_state)) - { - expectedType = ltRIPPLE_STATE; - Currency uCurrency; - Json::Value jvRippleState = context.params[jss::ripple_state]; + expectedType = ltPAYCHAN; - if (!jvRippleState.isObject() || - !jvRippleState.isMember(jss::currency) || - !jvRippleState.isMember(jss::accounts) || - !jvRippleState[jss::accounts].isArray() || - 2 != jvRippleState[jss::accounts].size() || - !jvRippleState[jss::accounts][0u].isString() || - !jvRippleState[jss::accounts][1u].isString() || - (jvRippleState[jss::accounts][0u].asString() == - jvRippleState[jss::accounts][1u].asString())) - { - jvResult[jss::error] = "malformedRequest"; - } - else - { - auto const id1 = parseBase58( - jvRippleState[jss::accounts][0u].asString()); - auto const id2 = parseBase58( - jvRippleState[jss::accounts][1u].asString()); - if (!id1 || !id2) + if (!uNodeIndex.parseHex( + context.params[jss::payment_channel].asString())) { - jvResult[jss::error] = "malformedAddress"; - } - else if ( - context.apiVersion > 1u && - !jvRippleState[jss::currency].isString()) - { - jvResult[jss::error] = "invalidParams"; + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; } - else if (!to_currency( - uCurrency, jvRippleState[jss::currency].asString())) + } + else if (context.params.isMember(jss::ripple_state)) + { + expectedType = ltRIPPLE_STATE; + Currency uCurrency; + Json::Value jvRippleState = context.params[jss::ripple_state]; + + if (!jvRippleState.isObject() || + !jvRippleState.isMember(jss::currency) || + !jvRippleState.isMember(jss::accounts) || + !jvRippleState[jss::accounts].isArray() || + 2 != jvRippleState[jss::accounts].size() || + !jvRippleState[jss::accounts][0u].isString() || + !jvRippleState[jss::accounts][1u].isString() || + (jvRippleState[jss::accounts][0u].asString() == + jvRippleState[jss::accounts][1u].asString())) { - jvResult[jss::error] = "malformedCurrency"; + jvResult[jss::error] = "malformedRequest"; } else { - uNodeIndex = keylet::line(*id1, *id2, uCurrency).key; + auto const id1 = parseBase58( + jvRippleState[jss::accounts][0u].asString()); + auto const id2 = parseBase58( + jvRippleState[jss::accounts][1u].asString()); + if (!id1 || !id2) + { + jvResult[jss::error] = "malformedAddress"; + } + else if (!to_currency( + uCurrency, + jvRippleState[jss::currency].asString())) + { + jvResult[jss::error] = "malformedCurrency"; + } + else + { + uNodeIndex = keylet::line(*id1, *id2, uCurrency).key; + } } } - } - else if (context.params.isMember(jss::ticket)) - { - expectedType = ltTICKET; - if (!context.params[jss::ticket].isObject()) + else if (context.params.isMember(jss::ticket)) { - if (context.apiVersion > 1u && - !context.params[jss::ticket].isString()) + expectedType = ltTICKET; + if (!context.params[jss::ticket].isObject()) { - jvResult[jss::error] = "invalidParams"; + if (!uNodeIndex.parseHex( + context.params[jss::ticket].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else if (!uNodeIndex.parseHex( - context.params[jss::ticket].asString())) + else if ( + !context.params[jss::ticket].isMember(jss::account) || + !context.params[jss::ticket].isMember(jss::ticket_seq) || + !context.params[jss::ticket][jss::ticket_seq].isIntegral()) { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } - } - else if ( - !context.params[jss::ticket].isMember(jss::account) || - !context.params[jss::ticket].isMember(jss::ticket_seq) || - !context.params[jss::ticket][jss::ticket_seq].isIntegral()) - { - jvResult[jss::error] = "malformedRequest"; - } - else if ( - context.apiVersion > 1u && - !context.params[jss::ticket][jss::account].isString()) - { - jvResult[jss::error] = "invalidParams"; - } - else - { - auto const id = parseBase58( - context.params[jss::ticket][jss::account].asString()); - if (!id) - jvResult[jss::error] = "malformedAddress"; else - uNodeIndex = getTicketIndex( - *id, context.params[jss::ticket][jss::ticket_seq].asUInt()); + { + auto const id = parseBase58( + context.params[jss::ticket][jss::account].asString()); + if (!id) + jvResult[jss::error] = "malformedAddress"; + else + uNodeIndex = getTicketIndex( + *id, + context.params[jss::ticket][jss::ticket_seq].asUInt()); + } } - } - else if (context.params.isMember(jss::nft_page)) - { - expectedType = ltNFTOKEN_PAGE; - - if (context.params[jss::nft_page].isString()) + else if (context.params.isMember(jss::nft_page)) { - if (!uNodeIndex.parseHex(context.params[jss::nft_page].asString())) + expectedType = ltNFTOKEN_PAGE; + + if (context.params[jss::nft_page].isString()) + { + if (!uNodeIndex.parseHex( + context.params[jss::nft_page].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } + } + else { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } } - else + else if (context.params.isMember(jss::amm)) { - jvResult[jss::error] = "malformedRequest"; - } - } - else if (context.params.isMember(jss::amm)) - { - expectedType = ltAMM; - if (!context.params[jss::amm].isObject()) - { - if (context.apiVersion > 1u && !context.params[jss::amm].isString()) + expectedType = ltAMM; + if (!context.params[jss::amm].isObject()) { - jvResult[jss::error] = "invalidParams"; + if (!uNodeIndex.parseHex(context.params[jss::amm].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - else if (!uNodeIndex.parseHex(context.params[jss::amm].asString())) + else if ( + !context.params[jss::amm].isMember(jss::asset) || + !context.params[jss::amm].isMember(jss::asset2)) { - uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; } - } - else if ( - !context.params[jss::amm].isMember(jss::asset) || - !context.params[jss::amm].isMember(jss::asset2)) - { - jvResult[jss::error] = "malformedRequest"; + else + { + try + { + auto const issue = + issueFromJson(context.params[jss::amm][jss::asset]); + auto const issue2 = + issueFromJson(context.params[jss::amm][jss::asset2]); + uNodeIndex = keylet::amm(issue, issue2).key; + } + catch (std::runtime_error const&) + { + jvResult[jss::error] = "malformedRequest"; + } + } } else { - try + if (context.params.isMember("params") && + context.params["params"].isArray() && + context.params["params"].size() == 1 && + context.params["params"][0u].isString()) { - auto const issue = - issueFromJson(context.params[jss::amm][jss::asset]); - auto const issue2 = - issueFromJson(context.params[jss::amm][jss::asset2]); - uNodeIndex = keylet::amm(issue, issue2).key; + if (!uNodeIndex.parseHex( + context.params["params"][0u].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } } - catch (std::runtime_error const&) + else { - jvResult[jss::error] = "malformedRequest"; + if (context.apiVersion < 2u) + jvResult[jss::error] = "unknownOption"; + else + jvResult[jss::error] = "invalidParams"; } } } - else + catch (Json::error& e) { - if (context.params.isMember("params") && - context.params["params"].isArray() && - context.params["params"].size() == 1 && - context.params["params"][0u].isString()) + if (context.apiVersion > 1u) { - if (!uNodeIndex.parseHex(context.params["params"][0u].asString())) - { - uNodeIndex = beast::zero; - jvResult[jss::error] = "malformedRequest"; - } + // For apiVersion 2 onwards, any parsing failures that throw this + // exception return an invalidParam error. + uNodeIndex = beast::zero; + jvResult[jss::error] = "invalidParams"; } else - { - if (context.apiVersion < 2u) - jvResult[jss::error] = "unknownOption"; - else - jvResult[jss::error] = "invalidParams"; - } + throw; } if (uNodeIndex.isNonZero())