hats-finance / OLD-Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

MIT License
0 stars 0 forks source link

depositWithSignature is allowed to be called by permit creator only #17

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf20d1e2e80b3c38ae27a81365a5880d7a19fa12d7ce1195ae792befaba7e411d Severity: medium

Description: Description\ depositWithSignature is allowed to be called by the permit creator only. No other contracts will be able to execute this function on behalf of the signer.

All the contracts in the staking folder have the same implementation, so the same issue persists across all.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File
function depositWithSignature(
    uint256 assets,
    address receiver,
    uint256 deadline,
    bool approveMax,
    uint8 v,
    bytes32 r,
    bytes32 s
) external nonReentrant returns (uint256 shares) {
    uint256 amount = approveMax ? type(uint256).max : assets;
    asset.permit(msg.sender, address(this), amount, deadline, v, r, s);
    return (deposit(assets, receiver));
}

function permit(
    address owner,
    address spender,
    uint256 value,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
) public virtual

As we can see msg.sender is always passed as owner. As a result, when someone with a valid permit signature wants to deposit on behalf of the owner he won’t be able to do so.

  1. Revised Code File (Optional)

Modify the code like this:

function depositWithSignature(
    uint256 assets,
+   address owner,
    address receiver,
    uint256 deadline,
    bool approveMax,
    uint8 v,
    bytes32 r,
    bytes32 s
) external nonReentrant returns (uint256 shares) {
    uint256 amount = approveMax ? type(uint256).max : assets;
-   asset.permit(msg.sender, address(this), amount, deadline, v, r, s);
+   asset.permit(owner, address(this), amount, deadline, v, r, s);
    return (deposit(assets, receiver));
}

This will allow everyone with a valid signature to deposit for the owner. This will be helpful when an owner has no gas to execute the transaction, which is the main idea of this EIP

0xRizwan commented 2 months ago

depositWithSignature() function is from staking contracts so its OOS.