## Relevant Context The Centrifuge protocol facilitates investments into RWAs from any integrated external EVM chain. Users in such external blockchains can interact with Centrifuge's infrastructure in order to invest and disinvest from the range of assets offered on Centrifuge. Furthermore, the tokenized versions of the offered RWAs are divided into different tranches, allowing customization of the risk-return profile of such assets for various investors. While the process of swapping user assets for tranche tokens is executed by submitting what are referred to as `investment order`s, the opposite action is executed by submitting `redemption order`s. ## Description Investment and redemption orders are processed by the Centrifuge protocol every time an epoch has passed. For the sake of this review, an epoch was assumed to last around `24h`. As an epoch is closed, all investment and redemption orders are accounted for by the [`OrderManager::process_invest_orders`](https://cantina.xyz/code/a0a58a8b-247e-4203-b3cb-476ded9d5515/pallets/investments/src/lib.rs?scope=in_scope&text=investment#L1149) and [`OrderManager::process_redeem_orders`](https://cantina.xyz/code/a0a58a8b-247e-4203-b3cb-476ded9d5515/pallets/investments/src/lib.rs?scope=in_scope&text=investment#L1198) methods, respectively. Such methods will simply move the value stored in `ActiveInvestOrders` and `ActiveRedeemOrders` into the `InProcessing*Orders` mappings. Once the orders have been written to their respective `InProcessing` mapping, the [`OrderManager::invest_fulfillment`](https://cantina.xyz/code/a0a58a8b-247e-4203-b3cb-476ded9d5515/pallets/investments/src/lib.rs?scope=in_scope&text=investment#L1247) and [`OrderManager::redeem_fulfillment`](https://cantina.xyz/code/a0a58a8b-247e-4203-b3cb-476ded9d5515/pallets/investments/src/lib.rs?scope=in_scope&text=investment#L1328) methods. Such methods are provided with a specific `investment_id`, which identifies a given tranche token, and a `fulfillment` struct, which specifies: 1. The percentage of the in processing orders that are being fulfilled with the current invocation, and 2. The price at which the fulfillment is being executed. These highlighted methods are responsible for, amongst other logic, calculating the required amount of assets or investment tokens to be moved, in correspondence of the specified amount of pending orders to be fulfilled: 1. In the case of investment order fulfillment, the method calculates the amount of assets pulled from the account tied to a given `investment_id`, which is where tokens waiting to be invested are held. Such amount of assets is thus transferred to the `investment_id`'s **account owner**. 2. In a similar fashion, the redemption order fulfillment executes the inverse operation: given the amount of tokens to be redeem, it calculates the amount of tokens to be pulled from the `investment_id`'s account owner and forwards them to the escrow account tied to `investment_id`. Crucially, knowing that an `investment_id`'s account owner isn't expected to hold any particular amount of assets in his account, nor is he required, an issue can arise in the case in which the redemption order fulfillment flow attempts to pull more tokens than such account's balance. This observation lead to finding a double vector which would lead to investment redemptions being bricked within the system: 1. Redemptions are executed at a higher price (**weighted over the fulfilled amounts**) than that of investments: this case would naturally lead to a consumption of the entire balance of the owner of an invesment asset's account, as repaying investments at a higher price will naturally require an increased amount of tokens with regards to those which were received during the investment order fulfillment process. 2. A malicious `investment_id` account owner may willingly hold a low balance of tokens in order to manipulate the price at which redemptions can be executed, easily manipulating the price of tranche tokens in his favor. In this case, while users aren't forbidden to sell their tranche tokens via the orderbook, they'd have no certainty of order fulfillment by an LP or other users, as the ability to redeem such tranche tokens has been compromised. As a result of the two scenarios presented above, users are left holding tranche tokens while, in the former case being unable to collect their fully earned profits, and in the latter case with no certainty to ever recover their initial investment. ## Impact Explanation High. The inability for the protocol to fulfill redemptions poses a considerable risk in its normal operations, which results in users being unable to convert tranche tokens into other assets. Such fault may lead users to sell their tranche tokens via the Centrifuge orderbook, causing a disruption of the investment and redemption accounting system and potentially driving the price of tranche tokens lower, as users scramble to exit their investments. Furthermore, blocking the redemption fulfillment process can lead to also a block in the investment fulfillment process in the case in which they are processed synchronously. ## Likelihood Explanation Medium. Given that tranche tokens are intended to reflect different risk levels for a given RWA, whose value may increase or decrease following conditions and movements, estimating the likelihood of a specific tranche token's price increasing or decreasing over time proves rather complicated. Hence, selecting a `Middle` likelihood seems the most reasonable choice, given that it represents a central value in the likelihood distribution. ## PoC Add the following test case to `pallets/investments/src/tests.rs` and execute it via `cargo test --release -p pallet-investments fulfillment_flow_increasing_price_poc` Please note that, given that the `TestExternalitiesBuilder::build()` mock environment builder assigns `100` tokens to all involved accounts, an extremely high price has been used when fulfilling the redemption orders in order to trigger the cited `BalanceTooLow` error. In production, the price change which triggers the issue at hand can be substantially lower, given that no amount of tokens is inteded to be held by the `investment_account` account as means of a liquidity buffer cover such liquidity requirements. ```rust #[test] fn fulfillment_flow_increasing_price_poc() { TestExternalitiesBuilder::build().execute_with(|| { #[allow(non_snake_case)] let PRICE: Quantity = price_of(1, 2, 10); #[allow(non_snake_case)] let SINGLE_REDEEM_AMOUNT = 50 * CURRENCY; #[allow(non_snake_case)] let TOTAL_REDEEM_AMOUNT = 3 * SINGLE_REDEEM_AMOUNT; #[allow(non_snake_case)] let SINGLE_INVEST_AMOUNT = 50 * CURRENCY; #[allow(non_snake_case)] let TOTAL_INVEST_AMOUNT = 3 * SINGLE_INVEST_AMOUNT; // Setup investments { assert_ok!(invest_x_per_investor(SINGLE_REDEEM_AMOUNT)); assert_ok!(redeem_x_per_investor(SINGLE_INVEST_AMOUNT)); } // calling orders increases order id and puts orders into // processing. Active orders a reset correctly { let invest_orders = Investments::process_invest_orders(INVESTMENT_0_0).expect("Did not call it twice"); assert_eq!(InvestOrderId::::get(INVESTMENT_0_0), 1); assert_eq! { invest_orders, TotalOrder{ amount: TOTAL_INVEST_AMOUNT} }; assert_eq! { InProcessingInvestOrders::::get(INVESTMENT_0_0), Some(TotalOrder { amount: TOTAL_INVEST_AMOUNT}) }; assert_eq! {ActiveInvestOrders::::get(INVESTMENT_0_0), TotalOrder{amount: 0}}; assert_eq! { last_event(), Event::InvestOrdersInProcessing { investment_id: INVESTMENT_0_0, order_id: 0, total_order: TotalOrder { amount: TOTAL_INVEST_AMOUNT} }.into() } } // Calling fulfillment on investments works { let fulfillment = FulfillmentWithPrice { of_amount: Perquintill::one(), price: PRICE, }; assert_ok!(Investments::invest_fulfillment(INVESTMENT_0_0, fulfillment)); assert_eq!( last_event(), Event::::InvestOrdersCleared { investment_id: INVESTMENT_0_0, order_id: 0, fulfillment } .into() ); assert_eq!( InProcessingInvestOrders::::get(INVESTMENT_0_0), None ); assert_eq!( ClearedInvestOrders::::get(INVESTMENT_0_0, 0), Some(fulfillment) ); assert_eq!( ActiveInvestOrders::::get(INVESTMENT_0_0), TotalOrder::default() ); } // checking balances have changed correctly { assert_eq!( free_balance_of(Owner::get(), AUSD_CURRENCY_ID), TOTAL_INVEST_AMOUNT + OWNER_START_BALANCE + ExistentialDeposit::get() ); assert_eq!( free_balance_of(investment_account(INVESTMENT_0_0), AUSD_CURRENCY_ID), 0 ); assert_eq!( free_balance_of(investment_account(INVESTMENT_0_0), INVESTMENT_0_0.into()), PRICE .reciprocal() .expect("Price is larger equal 1") .checked_mul_int(TOTAL_INVEST_AMOUNT) .expect("Unwrapping test checked_mul_int must work") // We need to take into account that the 3 TrancheHolders have submitted redeem // orders already .checked_add(TOTAL_REDEEM_AMOUNT) .expect("Unwrapping test checked_add must work") ) } // calling orders increases order id and puts orders into // processing. Active orders a reset correctly { let redeem_orders = Investments::process_redeem_orders(INVESTMENT_0_0).expect("Did not call it twice"); assert_eq!(RedeemOrderId::::get(INVESTMENT_0_0), 1); assert_eq! { redeem_orders, TotalOrder{ amount: TOTAL_REDEEM_AMOUNT} }; assert_eq! { InProcessingRedeemOrders::::get(INVESTMENT_0_0), Some(TotalOrder { amount: TOTAL_REDEEM_AMOUNT}) }; assert_eq! {ActiveRedeemOrders::::get(INVESTMENT_0_0), TotalOrder{amount: 0}}; assert_eq! { last_event(), Event::RedeemOrdersInProcessing { investment_id: INVESTMENT_0_0, order_id: 0, total_order: TotalOrder { amount: TOTAL_REDEEM_AMOUNT} }.into() } } // Calling fulfillment on redeem orders works { let fulfillment = FulfillmentWithPrice { of_amount: Perquintill::one(), price: price_of(1_000_000, 2, 10), // AUDIT redemptions are fulfilled at a higher price, in this case it is an extreme price which eats into the investment account's initial token balance }; assert_noop!( // AUDIT fulfillment fails because too many funds are attempted to be transferred Investments::redeem_fulfillment(INVESTMENT_0_0, fulfillment), orml_tokens::Error::::BalanceTooLow ); } }) } ``` ## Recommendation The Centrifuge protocol should consider enforcing a dynamic, minimum liquidity deposit on behalf of owner of `investment_id` accounts which is to be respected at all times. Such threshold would ensure a minimum price for redemption fulfillment execution. This threshold may be enforced by requiring the necessary amount to be held in escrow by leveraging the `Currency::hold` method. Furthermore, the redemption order fulfillment flow should take into consideration such token buffer exists, and fulfilling the maximum amount of tokens taken from the requested fulfillment and the liquidity buffer's availability.

Drastic Watermelon

Private reviews