Skip to content

Commit

Permalink
Fix/oak neutron audit issue 3 (#239)
Browse files Browse the repository at this point in the history
* Test case from audit.

* Use internally tracked balances for borrow amount check.

* Reorder interest rate updates. It should be executed after updating internally tracked balances.

* Add test case for interest rates.

---------

Co-authored-by: Pacman <pacman@apollo.farm>
  • Loading branch information
piobab and pacmanifold committed Jul 19, 2023
1 parent 8ccb3e9 commit 1b8e0c3
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 15 deletions.
2 changes: 1 addition & 1 deletion contracts/red-bank/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub enum ContractError {
#[error("Cannot have 0 as liquidity index")]
InvalidLiquidityIndex {},

#[error("Borrow amount must be greater than 0 {denom:?}")]
#[error("Borrow amount must be greater than 0 and less or equal available collateral (asset: {denom:?})")]
InvalidBorrowAmount {
denom: String,
},
Expand Down
30 changes: 19 additions & 11 deletions contracts/red-bank/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,6 @@ pub fn deposit(
response,
)?;

response = update_interest_rates(&env, &mut market, response)?;

if market.liquidity_index.is_zero() {
return Err(ContractError::InvalidLiquidityIndex {});
}
Expand All @@ -422,6 +420,9 @@ pub fn deposit(
)?;

market.increase_collateral(deposit_amount_scaled)?;

response = update_interest_rates(&env, &mut market, response)?;

MARKETS.save(deps.storage, &denom, &market)?;

Ok(response
Expand Down Expand Up @@ -517,8 +518,6 @@ pub fn withdraw(
response,
)?;

response = update_interest_rates(&env, &mut market, response)?;

// reduce the withdrawer's scaled collateral amount
let withdrawer_balance_after = withdrawer_balance_before.checked_sub(withdraw_amount)?;
let withdrawer_balance_scaled_after =
Expand All @@ -536,6 +535,9 @@ pub fn withdraw(
)?;

market.decrease_collateral(withdraw_amount_scaled)?;

response = update_interest_rates(&env, &mut market, response)?;

MARKETS.save(deps.storage, &denom, &market)?;

// send underlying asset to user or another recipient
Expand Down Expand Up @@ -566,13 +568,6 @@ pub fn borrow(
) -> Result<Response, ContractError> {
let borrower = User(&info.sender);

// Cannot borrow zero amount
if borrow_amount.is_zero() {
return Err(ContractError::InvalidBorrowAmount {
denom,
});
}

// Load market and user state
let mut borrow_market = MARKETS.load(deps.storage, &denom)?;

Expand All @@ -582,6 +577,19 @@ pub fn borrow(
});
}

let collateral_balance_before = get_underlying_liquidity_amount(
borrow_market.collateral_total_scaled,
&borrow_market,
env.block.time.seconds(),
)?;

// Cannot borrow zero amount or more than available collateral
if borrow_amount.is_zero() || borrow_amount > collateral_balance_before {
return Err(ContractError::InvalidBorrowAmount {
denom,
});
}

let uncollateralized_loan_limit = borrower.uncollateralized_loan_limit(deps.storage, &denom)?;

let config = CONFIG.load(deps.storage)?;
Expand Down
10 changes: 10 additions & 0 deletions contracts/red-bank/tests/test_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ fn borrow_and_repay() {
borrow_rate: Decimal::from_ratio(20u128, 100u128),
liquidity_rate: Decimal::from_ratio(10u128, 100u128),
reserve_factor: Decimal::from_ratio(1u128, 100u128),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
indexes_last_updated: 10000000,
..Default::default()
};
let mock_market_2 = Market {
borrow_index: Decimal::one(),
liquidity_index: Decimal::one(),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
..Default::default()
};
let mock_market_3 = Market {
Expand Down Expand Up @@ -481,6 +483,7 @@ fn repay_without_refund_on_behalf_of() {
liquidity_index: Decimal::one(),
borrow_index: Decimal::one(),
max_loan_to_value: Decimal::from_ratio(50u128, 100u128),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
..Default::default()
};

Expand Down Expand Up @@ -561,6 +564,7 @@ fn repay_with_refund_on_behalf_of() {
liquidity_index: Decimal::one(),
borrow_index: Decimal::one(),
max_loan_to_value: Decimal::from_ratio(50u128, 100u128),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
..Default::default()
};

Expand Down Expand Up @@ -672,6 +676,7 @@ fn borrow_uusd() {
borrow_index: Decimal::from_ratio(20u128, 10u128),
borrow_rate: Decimal::one(),
liquidity_rate: Decimal::one(),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
indexes_last_updated: block_time,
..Default::default()
Expand Down Expand Up @@ -750,6 +755,7 @@ fn borrow_full_liquidity_and_then_repay() {
borrow_index: Decimal::one(),
borrow_rate: Decimal::one(),
liquidity_rate: Decimal::one(),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
reserve_factor: Decimal::from_ratio(12u128, 100u128),
indexes_last_updated: block_time,
Expand Down Expand Up @@ -826,20 +832,23 @@ fn borrow_collateral_check() {

let mock_market_1 = Market {
max_loan_to_value: Decimal::from_ratio(8u128, 10u128),
collateral_total_scaled: Uint128::new(10_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
liquidity_index: Decimal::one(),
borrow_index: Decimal::from_ratio(1u128, 2u128),
..Default::default()
};
let mock_market_2 = Market {
max_loan_to_value: Decimal::from_ratio(6u128, 10u128),
collateral_total_scaled: Uint128::new(10_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
liquidity_index: Decimal::one(),
borrow_index: Decimal::from_ratio(1u128, 2u128),
..Default::default()
};
let mock_market_3 = Market {
max_loan_to_value: Decimal::from_ratio(4u128, 10u128),
collateral_total_scaled: Uint128::new(10_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
liquidity_index: Decimal::one(),
borrow_index: Decimal::from_ratio(1u128, 2u128),
Expand Down Expand Up @@ -954,6 +963,7 @@ fn borrow_and_send_funds_to_another_user() {
liquidity_index: Decimal::one(),
borrow_index: Decimal::one(),
max_loan_to_value: Decimal::from_ratio(5u128, 10u128),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
..Default::default()
};
Expand Down
1 change: 1 addition & 0 deletions contracts/red-bank/tests/test_misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ fn uncollateralized_loan_limits() {
borrow_rate: Decimal::from_ratio(20u128, 100u128),
liquidity_rate: Decimal::from_ratio(10u128, 100u128),
reserve_factor: Decimal::from_ratio(1u128, 10u128),
collateral_total_scaled: Uint128::new(1_000_000_000_000u128),
debt_total_scaled: Uint128::zero(),
indexes_last_updated: 10000000,
..Default::default()
Expand Down
8 changes: 6 additions & 2 deletions integration-tests/tests/test_rover_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ fn rover_flow() {

let rover = Addr::unchecked("rover");

// fund red-bank and set credit line for rover
// deposit more than credit line
let rover_uusdc_limit = 1_000_000_000_000u128;
mock_env.fund_account(&red_bank.contract_addr, &[coin(rover_uusdc_limit, "uusdc")]);
let uusdc_deposited = rover_uusdc_limit + 100_000u128;
mock_env.fund_account(&owner, &[coin(uusdc_deposited, "uusdc")]);
red_bank.deposit(&mut mock_env, &owner, coin(uusdc_deposited, "uusdc")).unwrap();

// set credit line for rover
red_bank
.update_uncollateralized_loan_limit(
&mut mock_env,
Expand Down
Loading

0 comments on commit 1b8e0c3

Please sign in to comment.