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

anyone can approve tokens from CapitalPool to TokenManager

dinkras

Summary

Vulnerability Details

Only the TokenManager contract is expected to call CapitalPool.approve(..) according to the docs

/**
* @dev Approve token for token manager
* @notice only can be called by token manager
* @param tokenAddr address of token
*/
function approve(address tokenAddr) external {

However there is not check for msg.sender so practically anyone can call this function whenever he wants for whatever token he wants.

Code snippet:

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/CapitalPool.sol#L24-L27

Impact

Invariant break, only token manager is supposed to call approve()

Tools Used

Manual review

Recommendations

Add check if the caller of CapitalPool.approve() is the token manager contract

function approve(address tokenAddr) external {//@audit-issue tokenManager calls this.approve with his address instead of the WETH token!
address tokenManager = tadleFactory.relatedContracts(
RelatedContractLibraries.TOKEN_MANAGER
);
+ requre(msg.sender == tokenManager, "Caller not TokenManager")
​
(bool success, ) = tokenAddr.call(//@audit-issue 1 : titple CapitalPool:approve should use OZ forceApprove. Tokens like USDT will revert when approve(x !=0) is called and there is an existing allowance
abi.encodeWithSelector(
APPROVE_SELECTOR,// q
tokenManager,
type(uint256).max
)
);//@audit-issue 2 return value falce not handled! EIP states that all return values false should be handled: Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!
​
if (!success) {
revert ApproveFailed();
}
}
​
Updates

Lead Judging Commences

0xnevi Lead Judge 10 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.