sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

Tri-pathi - Funds will lost if treasury is blacklisted #938

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

Tri-pathi

medium

Funds will lost if treasury is blacklisted

Funds will lost if treasury is blacklisted

Vulnerability Detail

Some tokens have blacklisted functions and If this address is blacklisted by the token then address becomes unable to make transfers, leading to funds being stuck in the address indefinitely.

whenever new pool is created or someone send funds to pool , base fee and other transfer Fee send to treasury address

File: contracts/core/Allo.sol
[...............]
476    _transferAmount(NATIVE, treasury, baseFee);
[..............]
516    _transferAmountFrom(_token, TransferData({from: msg.sender, to: address(_strategy), amount: amountAfterFee}));

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Allo.sol#L481 https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Allo.sol#L516

funds sent to this contract will be lost if token is blacklisted by token. As there is a way to change treasury address but that won't help in getting funds back. The following impact is enough to make this Medium

Impact

  1. all funds sent to treasury will be lost
  2. ProfileOwner won't able to create new pool or fund pool untill Admin changes the treasury address

Code Snippet

https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Allo.sol#L481 https://github.com/allo-protocol/allo-v2/blob/0b881ef4a0013d2809374c9ea69f4cf1288dfe62/contracts/core/Allo.sol#L516

Tool used

Manual Review

Recommendation

Implement withdraw pattern as currently protocol recover funds to any address. Apply similar pattern to deduct fee whenever pool is created or pool are funded.

sherlock-admin commented 11 months ago

1 comment(s) were left on this issue during the judging contest.

n33k commented:

invalid, low likelihood and unavoidable of being blacklisted, can easily recover from DoS by change treasury