code-423n4 / 2022-08-olympus-findings

5 stars 4 forks source link

# ERC20 transfer / transferFrom with not checked return value #486

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/549b96bcf8b97807738572605f6b1e26b33ef411/src/policies/Governance.sol#L259 https://github.com/code-423n4/2022-08-olympus/blob/549b96bcf8b97807738572605f6b1e26b33ef411/src/policies/Governance.sol#L312

Vulnerability details

ERC20 transfer / transferFrom with not checked return value

Impact

Not every ERC20 token follows OpenZeppelin's recommendation. It's possible (inside ERC20 standard) that a transferFrom doesn't revert upon failure but returns false.

Code doesn't check return values.

Proof of Concept

ERC20 transferfrom
https://github.com/code-423n4/2022-08-olympus/blob/549b96bcf8b97807738572605f6b1e26b33ef411/src/policies/Governance.sol#L259 https://github.com/code-423n4/2022-08-olympus/blob/549b96bcf8b97807738572605f6b1e26b33ef411/src/policies/Governance.sol#L312

Recommended Mitigation Steps

Consider using OpenZeppelin's library with safe versions of transfer functions. Check return value / revert if needed.

bahurum commented 2 years ago

Seems invalid to me. Warden references to VOTES tokens, which should revert on failed transfer.

bahurum commented 2 years ago

duplicate of #358

0xLienid commented 2 years ago

Disputed for reason provided by bahurum

0xean commented 1 year ago

closing as invalid see - https://github.com/code-423n4/2022-08-olympus-findings/issues/358#issuecomment-1249605848