User can get more ERC20 token from the ERC20WrapperUpgradeable contract if, Fee on the Transfer token get used.

[code423n4]

Lines of code

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/dd8ca8adc47624c5c5e2f4d412f5f421951dcc25/contracts/token/ERC20/extensions/ERC20WrapperUpgradeable.sol#L55 https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/dd8ca8adc47624c5c5e2f4d412f5f421951dcc25/contracts/token/ERC20/extensions/ERC20WrapperUpgradeable.sol#L63

Vulnerability details

Impact

Some users will lose money from the ERC20WrapperUpgradeable contract if, Fee on the Transfer token get used.

Proof of Concept

Some tokens take a transfer fee (e.g. STA, PAXG), some do not currently charge a fee but may do so in the future (e.g. USDT, USDC).

Based on the devs.aragon.org/docs, Wraps an existing ERC-20 token by inheriting from ERC20WrapperUpgradeable and allows to use it for voting by inheriting from ERC20VotesUpgradeable.

There are two functions in the Implementation of ERC20WrapperUpgradeable, public function depositFor and public function withdrawTo.

By public function depositFor, the user can Deposits an amount of underlying token and mints the corresponding number of wrapped tokens for an receiving address.

If we use tokens that take a transfer fee ( for example USDT with a 1% fee on transfer ) on the ERC20WrapperUpgradeable, if the user sends an amount = 100 USDT to the ERC20WrapperUpgradeable contract, the contract will receive 99 USDT tokens. Then in the body of the function depositFor, _mint(account, amount); gets called to mint amount = 100 Wrapped tokens to the account address.

function depositFor( address account, uint256 amount ) public override(IGovernanceWrappedERC20, ERC20WrapperUpgradeable) returns (bool) { return ERC20WrapperUpgradeable.depositFor(account, amount); }

Now user deposited 99 USDT but got 100 ERC20WrapperUpgradeable tokens.

Then user decides to get back his USDT tokens. Now the user will call the function withdrawTo. public function withdrawTo, Withdraws an amount of underlying tokens to an receiving address and burns the corresponding number of wrapped tokens.

After calling withdrawTo by the user, the amount = 100 ERC20WrapperUpgradeable tokens from the user will get burned and the ERC20WrapperUpgradeable contract will transfer the amount = 100 USDT to the user. The user will get more tokens and these more tokens are from other users.

function withdrawTo( address account, uint256 amount ) public override(IGovernanceWrappedERC20, ERC20WrapperUpgradeable) returns (bool) { return ERC20WrapperUpgradeable.withdrawTo(account, amount);

}

And this can cause that some users lose money from the ERC20WrapperUpgradeable contract.

Tools Used

Manually

Recommended Mitigation Steps

use balance before and balance after pattern to calculate amount of tokens that should get mint for user.

[0xean]

Tempted to call this out of scope as the "vulnerability" as stated is in an OZ package and a DAO using a fee on transfer token would need to understand how to properly configure their fee on transfer token for use. Will leave open, but most likely this becomes QA.

[c4-judge]

0xean marked the issue as duplicate of #125

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[Simon-Busch]

Nullified this issue as this one: https://github.com/code-423n4/2023-03-aragon-findings/issues/125 contains the warden information