sherlock-audit / 2024-06-boost-aa-wallet-judging

3 stars 1 forks source link

Ironsidesec - ERC20 and ERC1155 tokens can be pulled from anyone during budget allocation #413

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

Ironsidesec

High

ERC20 and ERC1155 tokens can be pulled from anyone during budget allocation

Summary

Root cause : ERC115 approvals in most cases will be in _operatorApprovals by calling _setApprovalForAll, so if anyone approves to Mnaaged budget to spend his ERX1155 during allocate call, immediately someone will backrun and allocate more than necessary from the target address. And it is a loss of funds to the target. Issue is worse, if ERC20 tokens with infinite allowance, in that case all the balance of target can be allocated to managed budget.

Impact: loss of ERC1155 and ERC20 tokens to the target address.

Vulnerability Detail

Issue flow:

  1. Budget creator address A, approves an ERC1155 token's _setApprovalForAll to allow managed budget to be the operator.
  2. He has 100 erc1155 tokens, but wants to allocate only 50 of them. And he approve first in one block. And in the next transaction, he calls ManagedBudget.allocate with payload.amount == 50 and request.target = his address A.
  3. Attacker can do things as easy as, before the allowance is revoked, he can call ManagedBudget.allocate with payload.amount == 50 and request.target = his address A. So, now all 100 tokens are allocated, losing 50 tokens from the address A who only wanted to allocate 50.

The attacker can either backrun the token approval or wait to backrun the allocate call. Note, that no MEV is required to this attack, backrun means the attacker transaction happens in the immediate next block of victim's tx.

The issue is the same with ERC20, because most approvals are infinite approvals, and before calling revoke, the attacker will allocate all the address A's balance.

https://github.com/sherlock-audit/2024-06-boost-aa-wallet/blob/78930f2ed6570f30e356b5529bd4bcbe5194eb8b/boost-protocol/packages/evm/contracts/budgets/ManagedBudget.sol#L70-L79


ManagedBudget.sol

59:     function allocate(bytes calldata data_) external payable virtual override returns (bool) {
60:         Transfer memory request = abi.decode(data_, (Transfer));
61:         if (request.assetType == AssetType.ETH) {
 ---- SNIP ----

68:         } else if (request.assetType == AssetType.ERC20) {
 ---- SNIP ----

72:    >>>      request.asset.safeTransferFrom(request.target, address(this), payload.amount);
 ---- SNIP ----

76:         } else if (request.assetType == AssetType.ERC1155) {
77:             ERC1155Payload memory payload = abi.decode(request.data, (ERC1155Payload));
78: 
80:             IERC1155(request.asset).safeTransferFrom(
81:    >>>          request.target, address(this), payload.tokenId, payload.amount, payload.data
82:             );
 ---- SNIP ----
92:     }

Impact

ERC1155 and ERC20 tokens can be pulled from anyone with sufficient allowance to manager budget contracts. The likelihood is 100% on erc1155 tokens and <50% on the ERC20 tokens (required infinite allowance users) So, allocation is prone to this attack causing loss of funds to the target address who wants to allocate.

Code Snippet

https://github.com/sherlock-audit/2024-06-boost-aa-wallet/blob/78930f2ed6570f30e356b5529bd4bcbe5194eb8b/boost-protocol/packages/evm/contracts/budgets/ManagedBudget.sol#L70-L79

Tool used

Manual Review

Recommendation

Add a validation check to see if the msg.sender is request.target on ManagedBudget.allocate function.