Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

CapitalPool.sol :: approve() can be called by anyone, not just by TokenManager.

ivanfitro

Summary

approve() is used to allow the TokenManager to spend tokens on behalf of CapitalPool. However, despite the @notice stating that it can only be called by TokenManager, it can actually be called by anyone since it is an external function.

Vulnerability Details

approve() is implemented as follows.

/**
* @dev Approve token for token manager
* @notice only can be called by token manager
* @param tokenAddr address of token
*/
function approve(address tokenAddr) external {
address tokenManager = tadleFactory.relatedContracts(
RelatedContractLibraries.TOKEN_MANAGER
);
(bool success, ) = tokenAddr.call(
abi.encodeWithSelector(
APPROVE_SELECTOR,
tokenManager,
type(uint256).max
)
);
​
if (!success) {
revert ApproveFailed();
}
}

The @notice states that the function only can be called by token manager, but the function is external and doesn't include any checks to ensure that the caller is indeed the TokenManager. This means that anyone can invoke the function with any arbitrary ERC20 token and implement a custom approve(), potentially leading to unexpected behaviors.

Impact

Anyone can call approve(), which is not the intended functionality of the function. This allows for the use a custom approve() function to perform arbitrary actions, potentially leading to unexpected behaviors.

Tools Used

Manual review.

Recommendations

Ensure that msg.sender is the TokenManager contract.

function approve(address tokenAddr) external {
address tokenManager = tadleFactory.relatedContracts(
RelatedContractLibraries.TOKEN_MANAGER
);
​
+ require(msg.sender == tokenManager, "Not TokenManager");
(bool success, ) = tokenAddr.call(
abi.encodeWithSelector(
APPROVE_SELECTOR,
tokenManager,
type(uint256).max
)
);
​
if (!success) {
revert ApproveFailed();
}
}
Updates

Lead Judging Commences

0xnevi Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-CapitalPool-approve-missing-access-control

This is at most low severity, even though giving max approvals shouldn't be permisionless, the respective tokenManager address is retrieved from the TadleFactory contract whereby the trusted guardian role is responsible for deploying such contracts as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/factory/TadleFactory.sol#L68). Since the user still has to go through the PreMarkets/DeliveryPlace contracts to perform market actions, this max approval cannot be exploited.

Support

FAQs

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