Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

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

Open
sherlock-admin opened this issue Apr 30, 2023 · 6 comments
Open

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

sherlock-admin opened this issue Apr 30, 2023 · 6 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

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))
            )
        );
@github-actions github-actions bot closed this as completed May 3, 2023
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label May 3, 2023
@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label May 20, 2023
@Ch-301
Copy link

Ch-301 commented May 21, 2023

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
Copy link
Contributor Author

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
Copy link

hrishibhat commented Jun 1, 2023

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
Copy link
Contributor Author

sherlock-admin commented Jun 1, 2023

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.

@hrishibhat hrishibhat reopened this Jun 1, 2023
@sherlock-admin sherlock-admin added Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Jun 1, 2023
@hrishibhat hrishibhat added the Medium A valid Medium severity issue label Jun 1, 2023
@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Jun 1, 2023
@sherlock-admin sherlock-admin removed the Excluded Excluded by the judge without consulting the protocol or the senior label Jun 5, 2023
@Gornutz Gornutz added the Will Fix The sponsor confirmed this issue will be fixed label Jun 6, 2023
@IAm0x52
Copy link
Collaborator

IAm0x52 commented Jun 16, 2023

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

5 participants