`_safe_transfer_from` Is Implemented Incorrectly
Summary
bool returndata is not validated during calls to _safe_transfer_from(address,address,address,uint256).
Vulnerability Details
The Rescuable interface implemented by multiple core contracts defines a _safe_transfer_from(address,address,address,uint256) function which fails to validate bool returndata for ERC-20s, under the assumption that all failed transfers will result in a revert.
However, this case does not hold true for ERC-20s which do not reverton failed transfers, and instead return a boolindicative of transfer success.
This impacts the reliability of token transfers in the TokenManager:
Impact
Incompatibility with the NoRevert weird-erc20.
Tools Used
Manual Review
Recommendations
Use a standardized implementation of safe transfers, such as OpenZeppelin's SafeERC20. Alternatively, conditionally validate non-zero returndata for boolean success indicators.
Lead Judging Commences
[invalid] finding-weird-erc-20-return-boolean-Rescuable
I believe the issues and duplicates do not warrant low severity severity as even if the call to transfers returns false instead of reverting, there is no impact as it is arguably correct given there will be insufficient funds to perform a rescue/withdrawal. This will not affect `tillIn()` as there are explicit balance [checks that revert accordingly](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L255-L260) to prevent allowing creation of offers without posting the necessary collateral
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.