diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index e6689bc880..76c5fc5fa8 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -347,6 +347,7 @@ enum TECcodes : TERUnderlyingType { tecBAD_CREDENTIALS = 193, tecWRONG_ASSET = 194, tecLIMIT_EXCEEDED = 195, + tecREMOVING_PERMISSIONS = 196, }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 66d0391b9f..4778bc3613 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -479,6 +479,7 @@ TRANSACTION(ttVAULT_CREATE, 64, VaultCreate, ({ TRANSACTION(ttVAULT_SET, 65, VaultSet, ({ {sfVaultID, soeREQUIRED}, {sfAssetMaximum, soeOPTIONAL}, + {sfDomainID, soeOPTIONAL}, // PermissionedDomainID // no WithdrawalPolicy yet {sfData, soeOPTIONAL}, })) diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index b79917ac79..c4bb2d327e 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -119,6 +119,7 @@ transResults() MAKE_ERROR(tecBAD_CREDENTIALS, "Bad credentials."), MAKE_ERROR(tecWRONG_ASSET, "Wrong asset given."), MAKE_ERROR(tecLIMIT_EXCEEDED, "Limit exceeded."), + MAKE_ERROR(tecREMOVING_PERMISSIONS, "Would remove permissions previously granted."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 98b5fc7a79..8c00acb745 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace ripple { @@ -45,16 +46,19 @@ VaultCreate::preflight(PreflightContext const& ctx) return temSTRING_TOO_LARGE; } - auto const domain = ctx.tx[~sfDomainID]; - if (domain && *domain == beast::zero) - return temMALFORMED; + if (auto const domain = ctx.tx[~sfDomainID]) + { + if (*domain == beast::zero) + return temMALFORMED; + else if ((ctx.tx.getFlags() & tfVaultPrivate) == 0) + return temMALFORMED; // DomainID only allowed on private vaults + } - // This block is copied from `MPTokenIssuanceCreate::preflight`. if (auto const metadata = ctx.tx[~sfMPTokenMetadata]) { if (metadata->length() == 0 || metadata->length() > maxMPTokenMetadataLength) - return temSTRING_TOO_LARGE; + return temMALFORMED; } return preflight2(ctx); @@ -83,8 +87,7 @@ VaultCreate::preclaim(PreclaimContext const& ctx) return tecLOCKED; } - auto const domain = ctx.tx[~sfDomainID]; - if (domain) + if (auto const domain = ctx.tx[~sfDomainID]) { auto const sleDomain = ctx.view.read(keylet::permissionedDomain(*domain)); @@ -129,7 +132,7 @@ VaultCreate::doApply() auto txFlags = tx.getFlags(); std::uint32_t mptFlags = 0; - if (!(txFlags & tfVaultShareNonTransferable)) + if ((txFlags & tfVaultShareNonTransferable) == 0) mptFlags |= (lsfMPTCanEscrow | lsfMPTCanTrade | lsfMPTCanTransfer); if (txFlags & tfVaultPrivate) mptFlags |= lsfMPTRequireAuth; diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 28fddb2cf7..b4c2f86097 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -49,6 +50,19 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) auto const vault = ctx.view.read(keylet::vault(ctx.tx[sfVaultID])); if (!vault) return tecOBJECT_NOT_FOUND; + + // Only the VaultDeposit transaction is subject to this permission check. + if (vault->getFlags() == tfVaultPrivate && + ctx.tx[sfAccount] != vault->at(sfOwner)) + { + if (auto const domain = vault->at(~sfVaultID)) + { + if (!credentials::authorizedDomain( + ctx.view, *domain, ctx.tx[sfAccount])) + return tecNO_PERMISSION; + } + } + return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index cad4489c05..926dec88c7 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -20,7 +20,9 @@ #include #include #include +#include #include +#include #include namespace ripple { @@ -41,12 +43,40 @@ VaultSet::preflight(PreflightContext const& ctx) return temSTRING_TOO_LARGE; } + auto const domain = ctx.tx[~sfDomainID]; + if (domain && *domain == beast::zero) + return temMALFORMED; + return preflight2(ctx); } TER VaultSet::preclaim(PreclaimContext const& ctx) { + auto const id = ctx.tx[sfVaultID]; + auto const sle = ctx.view.read(keylet::vault(id)); + if (!sle) + return tecOBJECT_NOT_FOUND; + + // Assert that submitter is the Owner. + if (ctx.tx[sfAccount] != sle->at(sfOwner)) + return tecNO_PERMISSION; + + // We can only set domain if private flag was originally set and + // domain was not set + if (auto const domain = ctx.tx[~sfDomainID]) + { + if ((sle->getFlags() & tfVaultPrivate) == 0) + return tecREMOVING_PERMISSIONS; + if (auto const oldDomain = sle->at(~sfDomainID)) + { + if (*oldDomain != *domain) + return tecREMOVING_PERMISSIONS; + // else no change + } + // else domain wasn't set previously, we allow setting it now + } + return tesSUCCESS; } @@ -58,17 +88,12 @@ VaultSet::doApply() // we can consider downgrading them to `tef` or `tem`. auto const& tx = ctx_.tx; - auto const& owner = account_; // Update existing object. auto vault = view().peek({ltVAULT, tx[sfVaultID]}); if (!vault) return tecOBJECT_NOT_FOUND; - // Assert that submitter is the Owner. - if (owner != vault->at(sfOwner)) - return tecNO_PERMISSION; - // Update mutable flags and fields if given. if (tx.isFieldPresent(sfData)) vault->at(sfData) = tx[sfData]; @@ -78,6 +103,17 @@ VaultSet::doApply() return tecLIMIT_EXCEEDED; vault->at(sfAssetMaximum) = tx[sfAssetMaximum]; } + if (tx.isFieldPresent(sfDomainID)) + { + // In VaultSet::preclaim we enforce that either DomainID wasn't present + // in the vault, or was the same value as the one supplied. We also + // enforce that tfVaultPrivate must have been set in the vault. By + // adding DomainID to an existing private vault, we are allowing + // permissioned users to interract with a vault which was previously + // accessible to its owner only. We currently do not support making + // such a vault public (i.e. removal of tfVaultPrivate flag) + vault->setFieldH256(sfDomainID, tx.getFieldH256(sfDomainID)); + } view().update(vault); diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 49a36020ff..e5ced16afa 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -19,9 +19,12 @@ #include +#include #include #include +#include #include +#include #include namespace ripple { @@ -47,6 +50,7 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) auto const vault = ctx.view.read(keylet::vault(ctx.tx[sfVaultID])); if (!vault) return tecOBJECT_NOT_FOUND; + return tesSUCCESS; }