Revert deposit on behalf of

contracts/red-bank/src/contract.rs

@@ -50,9 +50,18 @@ pub fn execute(
         }
         ExecuteMsg::Deposit {
             account_id,
+            on_behalf_of,
         } => {
             let sent_coin = cw_utils::one_coin(&info)?;
-            deposit::deposit(deps, env, info, sent_coin.denom, sent_coin.amount, account_id)
+            deposit::deposit(
+                deps,
+                env,
+                info,
+                on_behalf_of,
+                sent_coin.denom,
+                sent_coin.amount,
+                account_id,
+            )
         }
         ExecuteMsg::Withdraw {
             denom,

contracts/red-bank/src/deposit.rs

@@ -1,6 +1,9 @@
-use cosmwasm_std::{DepsMut, Env, MessageInfo, Response, Uint128};
+use cosmwasm_std::{Addr, DepsMut, Env, MessageInfo, Response, Uint128};
 use mars_interest_rate::get_scaled_liquidity_amount;
-use mars_red_bank_types::address_provider::{self, MarsAddressType};
+use mars_red_bank_types::{
+    address_provider::{self, MarsAddressType},
+    error::MarsError,
+};
 
 use crate::{
     error::ContractError,
@@ -14,12 +17,11 @@ pub fn deposit(
     deps: DepsMut,
     env: Env,
     info: MessageInfo,
+    on_behalf_of: Option<String>,
     denom: String,
     deposit_amount: Uint128,
     account_id: Option<String>,
 ) -> Result<Response, ContractError> {
-    let mut market = MARKETS.load(deps.storage, &denom)?;
-
     let config = CONFIG.load(deps.storage)?;
 
     let addresses = address_provider::helpers::query_contract_addrs(
@@ -29,11 +31,32 @@ pub fn deposit(
             MarsAddressType::Incentives,
             MarsAddressType::RewardsCollector,
             MarsAddressType::Params,
+            MarsAddressType::CreditManager,
         ],
     )?;
     let rewards_collector_addr = &addresses[&MarsAddressType::RewardsCollector];
     let incentives_addr = &addresses[&MarsAddressType::Incentives];
     let params_addr = &addresses[&MarsAddressType::Params];
+    let credit_manager_addr = &addresses[&MarsAddressType::CreditManager];
+
+    let user_addr: Addr;
+    let user = match on_behalf_of.as_ref() {
+        // A malicious user can permanently disable the lend action in credit-manager contract by performing the following steps:
+        // 1.) Wait for a new asset XXX to be listed and makes sure there is no coin lent out for XXX from the credit-manager to red-bank.
+        // 2.) Calls deposit on red-bank and sends 1 XXX and deposits on behalf of credit-manager.
+        // 3.) A user wants to lend out XXX from credit-manager but the call fails because TOTAL_LENT_SHARES is never initialized
+        // because this query red_bank.query_lent(&deps.querier, &env.contract.address, &coin.denom)? returns one.
+        Some(address) if address == credit_manager_addr.as_str() => {
+            return Err(ContractError::Mars(MarsError::Unauthorized {}));
+        }
+        Some(address) => {
+            user_addr = deps.api.addr_validate(address)?;
+            User(&user_addr)
+        }
+        None => User(&info.sender),
+    };
+
+    let mut market = MARKETS.load(deps.storage, &denom)?;
 
     let asset_params = query_asset_params(&deps.querier, params_addr, &denom)?;
 
@@ -68,7 +91,7 @@ pub fn deposit(
     let deposit_amount_scaled =
         get_scaled_liquidity_amount(deposit_amount, &market, env.block.time.seconds())?;
 
-    response = User(&info.sender).increase_collateral(
+    response = user.increase_collateral(
         deps.storage,
         &market,
         deposit_amount_scaled,
@@ -86,6 +109,7 @@ pub fn deposit(
     Ok(response
         .add_attribute("action", "deposit")
         .add_attribute("sender", &info.sender)
+        .add_attribute("on_behalf_of", user)
         .add_attribute("denom", denom)
         .add_attribute("amount", deposit_amount)
         .add_attribute("amount_scaled", deposit_amount_scaled))

contracts/red-bank/tests/tests/test_deposit.rs

@@ -17,6 +17,7 @@ use mars_red_bank::{
 };
 use mars_red_bank_types::{
     address_provider::MarsAddressType,
+    error::MarsError,
     incentives,
     red_bank::{Collateral, ExecuteMsg, Market},
 };
@@ -113,6 +114,7 @@ fn depositing_with_no_coin_sent() {
         mock_info(depositor_addr.as_str(), &[]),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap_err();
@@ -135,6 +137,7 @@ fn depositing_with_multiple_coins_sent() {
         mock_info(depositor_addr.as_str(), &sent_coins),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap_err();
@@ -158,6 +161,7 @@ fn depositing_to_non_existent_market() {
         mock_info(depositor_addr.as_str(), &coins(123, false_denom)),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap_err();
@@ -195,6 +199,7 @@ fn depositing_to_disabled_market() {
         mock_info(depositor_addr.as_str(), &coins(123, denom)),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap_err();
@@ -252,6 +257,7 @@ fn depositing_above_cap(amount_to_deposit: u128, deposit_cap: u128, exp_ok: bool
         mock_info(depositor_addr.as_str(), &coins(amount_to_deposit, denom)),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     );
 
@@ -295,6 +301,7 @@ fn depositing_without_existing_position() {
         mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();
@@ -323,6 +330,7 @@ fn depositing_without_existing_position() {
         vec![
             attr("action", "deposit"),
             attr("sender", &depositor_addr),
+            attr("on_behalf_of", &depositor_addr),
             attr("denom", denom),
             attr("amount", deposit_amount.to_string()),
             attr("amount_scaled", expected_mint_amount),
@@ -385,6 +393,7 @@ fn depositing_with_existing_position() {
         mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();
@@ -421,3 +430,199 @@ fn depositing_with_existing_position() {
         }
     );
 }
+
+#[test]
+fn depositing_on_behalf_of() {
+    let TestSuite {
+        mut deps,
+        denom,
+        depositor_addr,
+        initial_market,
+    } = setup_test();
+
+    let deposit_amount = 123456u128;
+    let on_behalf_of_addr = Addr::unchecked("jake");
+
+    // compute expected market parameters
+    let block_time = 10000300;
+    let expected_params =
+        th_get_expected_indices_and_rates(&initial_market, block_time, Default::default());
+    let expected_mint_amount = compute_scaled_amount(
+        Uint128::from(deposit_amount),
+        expected_params.liquidity_index,
+        ScalingOperation::Truncate,
+    )
+    .unwrap();
+    let expected_reward_amount_scaled = compute_scaled_amount(
+        expected_params.protocol_rewards_to_distribute,
+        expected_params.liquidity_index,
+        ScalingOperation::Truncate,
+    )
+    .unwrap();
+
+    let res = execute(
+        deps.as_mut(),
+        mock_env_at_block_time(block_time),
+        mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
+        ExecuteMsg::Deposit {
+            account_id: None,
+            on_behalf_of: Some(on_behalf_of_addr.clone().into()),
+        },
+    )
+    .unwrap();
+
+    // NOTE: For this test, the accrued protocol reward is non-zero, so we do expect a message to
+    // update the index of the rewards collector.
+    assert_eq!(
+        res.messages,
+        vec![
+            SubMsg::new(WasmMsg::Execute {
+                contract_addr: MarsAddressType::Incentives.to_string(),
+                msg: to_binary(&incentives::ExecuteMsg::BalanceChange {
+                    user_addr: Addr::unchecked(MarsAddressType::RewardsCollector.to_string()),
+                    account_id: None,
+                    denom: initial_market.denom.clone(),
+                    user_amount_scaled_before: Uint128::zero(),
+                    total_amount_scaled_before: initial_market.collateral_total_scaled,
+                })
+                .unwrap(),
+                funds: vec![]
+            }),
+            SubMsg::new(WasmMsg::Execute {
+                contract_addr: MarsAddressType::Incentives.to_string(),
+                msg: to_binary(&incentives::ExecuteMsg::BalanceChange {
+                    user_addr: on_behalf_of_addr.clone(),
+                    account_id: None,
+                    denom: initial_market.denom.clone(),
+                    user_amount_scaled_before: Uint128::zero(),
+                    // NOTE: New collateral shares were minted to the rewards collector first, so
+                    // for the depositor this should be initial total supply + rewards shares minted
+                    total_amount_scaled_before: initial_market.collateral_total_scaled
+                        + expected_reward_amount_scaled,
+                })
+                .unwrap(),
+                funds: vec![]
+            })
+        ]
+    );
+
+    // depositor should not have created a new collateral position
+    let opt = COLLATERALS.may_load(deps.as_ref().storage, (&depositor_addr, "", denom)).unwrap();
+    assert!(opt.is_none());
+
+    // the recipient should have created a new collateral position
+    let collateral =
+        COLLATERALS.load(deps.as_ref().storage, (&on_behalf_of_addr, "", denom)).unwrap();
+    assert_eq!(
+        collateral,
+        Collateral {
+            amount_scaled: expected_mint_amount,
+            enabled: true,
+        }
+    );
+}
+
+#[test]
+fn depositing_on_behalf_of_cannot_enable_collateral() {
+    let TestSuite {
+        mut deps,
+        denom,
+        depositor_addr,
+        ..
+    } = setup_test();
+
+    deps.querier.set_oracle_price(denom, Decimal::one());
+
+    let on_behalf_of_addr = Addr::unchecked("jake");
+
+    let block_time = 10000300;
+
+    // 'on_behalf_of_addr' deposit funds to their own account
+    execute(
+        deps.as_mut(),
+        mock_env_at_block_time(block_time),
+        mock_info(on_behalf_of_addr.as_str(), &coins(1u128, denom)),
+        ExecuteMsg::Deposit {
+            account_id: None,
+            on_behalf_of: None,
+        },
+    )
+    .unwrap();
+
+    // 'on_behalf_of_addr' should have collateral enabled
+    let collateral =
+        COLLATERALS.load(deps.as_ref().storage, (&on_behalf_of_addr, "", denom)).unwrap();
+    assert!(collateral.enabled);
+
+    // 'on_behalf_of_addr' disables asset as collateral
+    execute(
+        deps.as_mut(),
+        mock_env_at_block_time(block_time),
+        mock_info(on_behalf_of_addr.as_str(), &[]),
+        ExecuteMsg::UpdateAssetCollateralStatus {
+            denom: denom.to_string(),
+            enable: false,
+        },
+    )
+    .unwrap();
+
+    // verify asset is disabled as collateral for 'on_behalf_of_addr'
+    let collateral =
+        COLLATERALS.load(deps.as_ref().storage, (&on_behalf_of_addr, "", denom)).unwrap();
+    assert!(!collateral.enabled);
+
+    // 'depositor_addr' deposits a small amount of funds to 'on_behalf_of_addr' to enable his asset as collateral
+    execute(
+        deps.as_mut(),
+        mock_env_at_block_time(block_time),
+        mock_info(depositor_addr.as_str(), &coins(1u128, denom)),
+        ExecuteMsg::Deposit {
+            account_id: None,
+            on_behalf_of: Some(on_behalf_of_addr.to_string()),
+        },
+    )
+    .unwrap();
+
+    // 'on_behalf_of_addr' doesn't have the asset enabled as collateral
+    let collateral =
+        COLLATERALS.load(deps.as_ref().storage, (&on_behalf_of_addr, "", denom)).unwrap();
+    assert!(!collateral.enabled);
+}
+
+#[test]
+fn depositing_on_behalf_of_credit_manager() {
+    let TestSuite {
+        mut deps,
+        denom,
+        depositor_addr,
+        ..
+    } = setup_test();
+
+    // disable the market
+    deps.querier.set_redbank_params(
+        denom,
+        AssetParams {
+            credit_manager: CmSettings {
+                whitelisted: false,
+                hls: None,
+            },
+            red_bank: RedBankSettings {
+                deposit_enabled: true,
+                borrow_enabled: true,
+            },
+            ..th_default_asset_params()
+        },
+    );
+
+    let err = execute(
+        deps.as_mut(),
+        mock_env(),
+        mock_info(depositor_addr.as_str(), &coins(123, denom)),
+        ExecuteMsg::Deposit {
+            account_id: None,
+            on_behalf_of: Some("credit_manager".to_string()),
+        },
+    )
+    .unwrap_err();
+    assert_eq!(err, ContractError::Mars(MarsError::Unauthorized {}));
+}

contracts/red-bank/tests/tests/test_liquidate.rs

@@ -746,6 +746,7 @@ fn response_verification() {
         mock_info(provider.as_str(), &[coin(1000000, "uusdc")]),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();
@@ -755,6 +756,7 @@ fn response_verification() {
         mock_info(provider.as_str(), &[coin(1000000, "untrn")]),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();
@@ -766,6 +768,7 @@ fn response_verification() {
         mock_info(liquidatee.as_str(), &[coin(10000, "uosmo")]),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();
@@ -775,6 +778,7 @@ fn response_verification() {
         mock_info(liquidatee.as_str(), &[coin(900, "uatom")]),
         ExecuteMsg::Deposit {
             account_id: None,
+            on_behalf_of: None,
         },
     )
     .unwrap();