sherlock-audit/2023-04-blueberry-judging

Ch_301 - asking for the wrong address for `balanceOf()`

Opened this issue · 6 comments

sherlock-admin commented

Ch_301

medium

asking for the wrong address for balanceOf()

Summary

Vulnerability Detail

ShortLongSpell.openPosition() pass to _doPutCollateral() wrong value of balanceOf()

        // 5. Put collateral - strategy token
        address vault = strategies[param.strategyId].vault;
        _doPutCollateral(
            vault,
            IERC20Upgradeable(ISoftVault(vault).uToken()).balanceOf(
                address(this)
            )
        );

the balance should be of address(vault)

Impact

  • openPosition() will never work

Code Snippet

Tool used

Manual Review

Recommendation

        // 5. Put collateral - strategy token
        address vault = strategies[param.strategyId].vault;
        _doPutCollateral(
            vault,
-            IERC20Upgradeable(ISoftVault(vault).uToken()).balanceOf(
-                address(this)
+                IERC20Upgradeable(vault).balanceOf(address(this))
            )
        );
Ch-301 commented

Escalate for 10 USDC

This is a simple finding when you know that SoftVault is transferring all uToken to Compound to generate yield

Also of wonder the judge set this as invalid but he submitted both this and #114 in the next contest Blueberry Update 2

  • 👍2
sherlock-admin commented

Escalate for 10 USDC

This is a simple finding when you know that SoftVault is transferring all uToken to Compound to generate yield

Also of wonder the judge set this as invalid but he submitted both this and #114 in the next contest Blueberry Update 2

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.

hrishibhat commented

Escalation accepted

Valid medium
Since the issue does not clearly identify the impact where the tokens can be stolen, but still correctly recognizes the underlying issue considering this a valid medium.

sherlock-admin commented

Escalation accepted

Valid medium
Since the issue does not clearly identify the impact where the tokens can be stolen, but still correctly recognizes the underlying issue considering this 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.
IAm0x52 commented

Fix looks good. _doPutCollateral now correctly uses the balance of the vault token rather than the balance of the underlying token