code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

ERC20 not checking for "to" field of address(0) in 3 functions #196

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L86 https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L100-L104 https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L196

Vulnerability details

Impact

Not checking if to == address(0) in transfer, transferFrom and _mint in the Erc20.sol might result in loosing/locking funds.

Proof of Concept

https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L86 https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L100-L104 https://github.com/code-423n4/2022-07-swivel/blob/main/Tokens/Erc20.sol#L196

Tools Used

editor

Recommended Mitigation Steps

Add an if or require statement which checks if the to address is the burn address (address(0)) and revert if so.

    if (to == address(0)) revert("ZERO_ADDRESS");
JTraversa commented 2 years ago

https://github.com/code-423n4/2022-07-swivel#input-sanitization

bghughes commented 2 years ago

https://github.com/code-423n4/2022-07-swivel#input-sanitization

Agreed, downgrading to QA - broader commentary on ERC-20 in some sense.

bghughes commented 2 years ago

Grouping this with the warden’s QA report, #188