sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

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

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

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

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 1 year ago

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

sherlock-admin commented 1 year ago

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 1 year ago

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 1 year ago

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.
Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/54e080a2d9dbfb2f44e8adcb715dde9261d3906e#diff-608b47c5c74c768d52e4449c12d1a3a9ce5d8ee01cce2e65df2e64996df651cb

IAm0x52 commented 1 year ago

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