sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

Ch_301 - Asking for `balanceOf()` in the wrong address #170

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

Ch_301

high

Asking for balanceOf() in the wrong address

Summary

Vulnerability Detail

on sendFundsToVault() this logic

address underlying = getUnderlyingAddress(_vaultNumber, _chain);
uint256 balance = IERC20(underlying).balanceOf(address(this));

in case _chainId is Optimism the underlying address is for Optimism (L2) but XChainController is on Mainnet you can't invoke balanceOf() like this!!!

Impact

Asking for balanceOf() in the wrong address The protocol will be not able to rebalance the vault

Code Snippet

Tool used

Manual Review

Recommendation

getUnderlyingAddress(_vaultNumber, _chain); should just be getUnderlyingAddress(_vaultNumber); so the underlying here

uint256 balance = IERC20(underlying).balanceOf(address(this));

will be always on the Mainnet

Ch-301 commented 1 year ago

Escalate for 10 USDC

This one was confirmed by the Sponsor check the discussion for more details image

sherlock-admin commented 1 year ago

Escalate for 10 USDC

This one was confirmed by the Sponsor check the discussion for more details image

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

Theezr commented 1 year ago

Valid medium issue

hrishibhat commented 1 year ago

Escalation accepted

Considering this issue a valid medium

sherlock-admin commented 1 year ago

Escalation accepted

Considering this issue a valid medium

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

Theezr commented 1 year ago

Fix: https://github.com/derbyfinance/derby-yield-optimiser/pull/224