Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Use abi.encodeCall() instead of abi.encodeWithSelector()

adeolu

Vulnerability Details

The low level call() in several functions use abi.encodeWithSelector() to encode the token function calldata. This function is not type safe. In a case where the arbitrary token address has a function with same name but different parameter types, the call() will still execute and not bubble up a revert, even though the action did not actually succeed. ERC20 tokens are so many and each have different variations. They may not all accept uint256 values like the hardcoded selectors in tadle contracts expect them to.

Snippet from CapitalPool.sol below

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L14-L15

bytes4 private constant APPROVE_SELECTOR =
bytes4(keccak256(bytes("approve(address,uint256)")));
​
​
function approve(address tokenAddr) external {
address tokenManager = tadleFactory.relatedContracts(
RelatedContractLibraries.TOKEN_MANAGER
);
(bool success, ) = tokenAddr.call(
abi.encodeWithSelector(
APPROVE_SELECTOR,
tokenManager,
type(uint256).max
)
);

Snippet from rescuable.solbelow

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/utils/Rescuable.sol#L13-L16

bytes4 private constant TRANSFER_SELECTOR =
bytes4(keccak256(bytes("transfer(address,uint256)")));
bytes4 private constant TRANSFER_FROM_SELECTOR =
bytes4(keccak256(bytes("transferFrom(address,address,uint256)")));
​
​
function _safe_transfer(
address token,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_SELECTOR, to, amount)
);
​
if (!success) {
revert TransferFailed();
}
}
​
/**
* @dev Safe transfer.
* @param token The token to transfer. If 0, it is ether.
* @param to The address of the account to transfer to.
* @param amount The amount to transfer.
*/
function _safe_transfer_from(
address token,
address from,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_FROM_SELECTOR, from, to, amount)
);
​
if (!success) {
revert TransferFailed();
}
}

Impact

if arbitray token functions take in parameters with slightly different types other than uint256 for example, with abi.encodeWithSelector the call() function will still run to completion and not return an error even though it failed.

Tools Used

manual review

Recommendations

Use abi.encodeCall() instead of abi.encodeWithSelector()

Updates

Lead Judging Commences

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.