code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

Return values of ERC20 transfer and transferFrom are unchecked #975

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L279 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L204 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L210 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L225 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L268 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L284 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L294 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L108 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L129 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L110 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L142 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L138 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L228 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L253 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L269 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/StablecoinBridge.sol#L45 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/StablecoinBridge.sol#L69

Vulnerability details

Impact

The return values of ERC20 transfer and transferFrom are not checked to be true, which could be false if the transferred tokens are not ERC20-compliant. In that case, the transfer fails without being noticed by the calling contract.

Proof of Concept

Links: https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L279 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L204 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L210 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L225 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L268 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L284 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L294 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L108 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L129 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L110 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/MintingHub.sol#L142 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L138 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L228 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L253 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L269 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/StablecoinBridge.sol#L45 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/StablecoinBridge.sol#L69

Tools Used

Manually

Recommended Mitigation Steps

Use the SafeERC20 library implementation from Openzeppelin and call safeTransfer or safeTransferFrom when transferring ERC20 tokens.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #777

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Invalid