From eb9f2eb85952c7d7b47749cd3411c3d64cd916d7 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Fri, 30 Jun 2023 13:40:34 -0700 Subject: [PATCH 1/4] Add Footprint LedgerKey validation --- src/transactions/TransactionFrame.cpp | 50 ++++++++++++++++++++--- src/transactions/TransactionFrame.h | 3 +- src/transactions/test/TxEnvelopeTests.cpp | 5 ++- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp index eb9e7518a2..78f5be5222 100644 --- a/src/transactions/TransactionFrame.cpp +++ b/src/transactions/TransactionFrame.cpp @@ -586,8 +586,8 @@ TransactionFrame::validateSorobanOpsConsistency() const } bool -TransactionFrame::validateSorobanResources( - SorobanNetworkConfig const& config) const +TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config, + uint32_t protocolVersion) const { auto const& resources = sorobanResources(); auto const& readEntries = resources.footprint.readOnly; @@ -615,16 +615,56 @@ TransactionFrame::validateSorobanResources( { return false; } + auto footprintKeyIsValid = [&](LedgerKey const& key) -> bool { + if (isSorobanExtEntry(key)) + { + return false; + } + + switch (key.type()) + { + case ACCOUNT: + case CONTRACT_DATA: + case CONTRACT_CODE: + break; + case TRUSTLINE: + { + auto const& tl = key.trustLine(); + if (!isAssetValid(tl.asset, protocolVersion) || + (tl.asset.type() == ASSET_TYPE_NATIVE) || + isIssuer(tl.accountID, tl.asset)) + { + return false; + } + break; + } + case OFFER: + case DATA: + case CLAIMABLE_BALANCE: + case LIQUIDITY_POOL: + case CONFIG_SETTING: + return false; + default: + throw std::runtime_error("unknown ledger key type"); + } + + if (xdr::xdr_size(key) > config.maxContractDataKeySizeBytes()) + { + return false; + } + + return true; + }; for (auto const& lk : readEntries) { - if (xdr::xdr_size(lk) > config.maxContractDataKeySizeBytes()) + if (!footprintKeyIsValid(lk)) { return false; } } for (auto const& lk : writeEntries) { - if (xdr::xdr_size(lk) > config.maxContractDataKeySizeBytes()) + if (!footprintKeyIsValid(lk)) { return false; } @@ -923,7 +963,7 @@ TransactionFrame::commonValidPreSeqNum(Application& app, AbstractLedgerTxn& ltx, } auto const& sorobanConfig = app.getLedgerManager().getSorobanNetworkConfig(ltx); - if (!validateSorobanResources(sorobanConfig)) + if (!validateSorobanResources(sorobanConfig, ledgerVersion)) { getResult().result.code(txSOROBAN_RESOURCE_LIMIT_EXCEEDED); return false; diff --git a/src/transactions/TransactionFrame.h b/src/transactions/TransactionFrame.h index d133cd0832..f3444fb6c9 100644 --- a/src/transactions/TransactionFrame.h +++ b/src/transactions/TransactionFrame.h @@ -136,7 +136,8 @@ class TransactionFrame : public TransactionFrameBase #ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION bool validateSorobanOpsConsistency() const; - bool validateSorobanResources(SorobanNetworkConfig const& config) const; + bool validateSorobanResources(SorobanNetworkConfig const& config, + uint32_t protocolVersion) const; void refundSorobanFee(uint32_t protocolVersion, SorobanNetworkConfig const& sorobanConfig, Config const& cfg, AbstractLedgerTxn& ltx); diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 93f50eff03..025132ac34 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2533,7 +2533,10 @@ TEST_CASE("soroban transaction validation", "[tx][envelope][soroban]") auto keys = LedgerTestUtils::generateValidUniqueLedgerEntryKeysWithExclusions( - {}, InitialSorobanNetworkConfig::TX_MAX_READ_LEDGER_ENTRIES); + {LedgerEntryType::OFFER, LedgerEntryType::DATA, + LedgerEntryType::CLAIMABLE_BALANCE, + LedgerEntryType::LIQUIDITY_POOL, LedgerEntryType::CONFIG_SETTING}, + InitialSorobanNetworkConfig::TX_MAX_READ_LEDGER_ENTRIES); resources.footprint.readWrite.assign( keys.begin(), From 486bcf71dde4540d77db2e9bfc9b7264513b9676 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Fri, 30 Jun 2023 13:41:46 -0700 Subject: [PATCH 2/4] Validate token contract Asset --- src/transactions/InvokeHostFunctionOpFrame.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/transactions/InvokeHostFunctionOpFrame.cpp b/src/transactions/InvokeHostFunctionOpFrame.cpp index eb55a1b129..5db3c4ee60 100644 --- a/src/transactions/InvokeHostFunctionOpFrame.cpp +++ b/src/transactions/InvokeHostFunctionOpFrame.cpp @@ -507,6 +507,15 @@ InvokeHostFunctionOpFrame::doCheckValid(SorobanNetworkConfig const& config, { return false; } + if (hostFn.type() == HOST_FUNCTION_TYPE_CREATE_CONTRACT) + { + auto const& preimage = hostFn.createContract().contractIDPreimage; + if (preimage.type() == CONTRACT_ID_PREIMAGE_FROM_ASSET && + !isAssetValid(preimage.fromAsset(), ledgerVersion)) + { + return false; + } + } return true; } From 62dd8f172656433002b7b31810b2bba34d4fdd82 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Fri, 30 Jun 2023 13:42:47 -0700 Subject: [PATCH 3/4] cleanup --- src/transactions/RevokeSponsorshipOpFrame.cpp | 3 ++- src/transactions/SponsorshipUtils.cpp | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/transactions/RevokeSponsorshipOpFrame.cpp b/src/transactions/RevokeSponsorshipOpFrame.cpp index 3f57591f2d..948a1953b8 100644 --- a/src/transactions/RevokeSponsorshipOpFrame.cpp +++ b/src/transactions/RevokeSponsorshipOpFrame.cpp @@ -45,7 +45,7 @@ getAccountID(LedgerEntry const& le) case CLAIMABLE_BALANCE: return *le.ext.v1().sponsoringID; default: - abort(); + throw std::runtime_error("Invalid key type"); } } @@ -434,6 +434,7 @@ RevokeSponsorshipOpFrame::doCheckValid(uint32_t ledgerVersion) case LIQUIDITY_POOL: #ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION case CONTRACT_DATA: + case CONTRACT_CODE: case CONFIG_SETTING: #endif innerResult().code(REVOKE_SPONSORSHIP_MALFORMED); diff --git a/src/transactions/SponsorshipUtils.cpp b/src/transactions/SponsorshipUtils.cpp index ee08e503a0..eb8eba0bda 100644 --- a/src/transactions/SponsorshipUtils.cpp +++ b/src/transactions/SponsorshipUtils.cpp @@ -205,6 +205,7 @@ computeMultiplier(LedgerEntry const& le) #ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION case CONFIG_SETTING: case CONTRACT_DATA: + case CONTRACT_CODE: #endif case LIQUIDITY_POOL: throw std::runtime_error( @@ -229,6 +230,7 @@ isSubentry(LedgerEntry const& le) #ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION case CONTRACT_DATA: case CONFIG_SETTING: + case CONTRACT_CODE: #endif case LIQUIDITY_POOL: throw std::runtime_error( From 3d206b568b58820a20edc5d0d3afb7512c04e189 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Fri, 30 Jun 2023 15:40:53 -0700 Subject: [PATCH 4/4] Fix tests --- src/herder/test/HerderTests.cpp | 12 ++++++------ src/ledger/test/LedgerTestUtils.cpp | 8 ++++++++ src/ledger/test/LedgerTestUtils.h | 4 ++++ src/transactions/test/TxEnvelopeTests.cpp | 8 ++------ 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index 2dc9a7783c..af4d32bff4 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -1619,15 +1619,15 @@ TEST_CASE("surge pricing", "[herder][txset]") auto write = rand_uniform( 0, std::min(conf.txMaxWriteLedgerEntries(), (conf.txMaxReadLedgerEntries() - read))); - for (auto const& key : LedgerTestUtils:: - generateValidLedgerEntryKeysWithExclusions( - {CONFIG_SETTING}, write)) + for (auto const& key : + LedgerTestUtils::generateUniqueValidSorobanLedgerEntryKeys( + write)) { res.footprint.readWrite.emplace_back(key); } - for (auto const& key : LedgerTestUtils:: - generateValidLedgerEntryKeysWithExclusions( - {CONFIG_SETTING}, read)) + for (auto const& key : + LedgerTestUtils::generateUniqueValidSorobanLedgerEntryKeys( + read)) { res.footprint.readOnly.emplace_back(key); } diff --git a/src/ledger/test/LedgerTestUtils.cpp b/src/ledger/test/LedgerTestUtils.cpp index 3320d5547f..12629e3cfc 100644 --- a/src/ledger/test/LedgerTestUtils.cpp +++ b/src/ledger/test/LedgerTestUtils.cpp @@ -635,6 +635,14 @@ generateValidLedgerEntryKeysWithExclusions( } return keys; } +#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION +std::vector +generateUniqueValidSorobanLedgerEntryKeys(size_t n) +{ + return LedgerTestUtils::generateValidUniqueLedgerEntryKeysWithExclusions( + {OFFER, DATA, CLAIMABLE_BALANCE, LIQUIDITY_POOL, CONFIG_SETTING}, n); +} +#endif std::vector generateValidUniqueLedgerEntryKeysWithExclusions( diff --git a/src/ledger/test/LedgerTestUtils.h b/src/ledger/test/LedgerTestUtils.h index afca79741a..6cc63a4d88 100644 --- a/src/ledger/test/LedgerTestUtils.h +++ b/src/ledger/test/LedgerTestUtils.h @@ -46,6 +46,10 @@ std::vector generateValidUniqueLedgerEntries(size_t n); std::vector generateValidLedgerEntryKeysWithExclusions( std::unordered_set const& excludedTypes, size_t n); +#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION +std::vector generateUniqueValidSorobanLedgerEntryKeys(size_t n); +#endif + std::vector generateValidUniqueLedgerEntryKeysWithExclusions( std::unordered_set const& excludedTypes, size_t n); diff --git a/src/transactions/test/TxEnvelopeTests.cpp b/src/transactions/test/TxEnvelopeTests.cpp index 025132ac34..304c7d727c 100644 --- a/src/transactions/test/TxEnvelopeTests.cpp +++ b/src/transactions/test/TxEnvelopeTests.cpp @@ -2531,12 +2531,8 @@ TEST_CASE("soroban transaction validation", "[tx][envelope][soroban]") resources.extendedMetaDataSizeBytes = InitialSorobanNetworkConfig::TX_MAX_EXTENDED_META_DATA_SIZE_BYTES; - auto keys = - LedgerTestUtils::generateValidUniqueLedgerEntryKeysWithExclusions( - {LedgerEntryType::OFFER, LedgerEntryType::DATA, - LedgerEntryType::CLAIMABLE_BALANCE, - LedgerEntryType::LIQUIDITY_POOL, LedgerEntryType::CONFIG_SETTING}, - InitialSorobanNetworkConfig::TX_MAX_READ_LEDGER_ENTRIES); + auto keys = LedgerTestUtils::generateUniqueValidSorobanLedgerEntryKeys( + InitialSorobanNetworkConfig::TX_MAX_READ_LEDGER_ENTRIES); resources.footprint.readWrite.assign( keys.begin(),