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

[Low-01] Missing Access Control in `CapitalPool::approve()` Function Allows any User to call it to set Allowance Amount `TokenContract` to `type(uint256).max`.

justawanderkid

Description

The approve() function in the CapitalPool contract lacks access control, allowing any user to call it. This function sets an unlimited token allowance for the TokenManager on a specified token address, enabling unauthorized approvals. natspec of CapitalPool::approve() function states that only can be called by token manager, but that's not the case here since there's no Modifiers that check the msg.sender address to match with TokenManager contract address.

Impact

Unauthorized user can call the CapitalPool::approve() function, which breaks the invariant where approve() function can only be called as TokenManager contract which is stated in natspec of the function.

Proof of Concepts

Random User Comes and calls CapitalPool::approve() function which Gives TokenManager Contract Approval to Spend type(uint256).max Amount.

function test_anyoneCanCallApproveFunction() external {
vm.startPrank(user);
capitalPool.approve(address(mockUSDCToken));
vm.stopPrank();
​
assertEq(mockUSDCToken.allowance(address(capitalPool), address(tokenManager)), type(uint256).max);
​
console2.log("This is type(uint256)max:");
console2.log(" ", type(uint256).max);
​
console2.log("This is The Allowance Amount the 'CapitalPool' Contract Gaved to `TokenContract`:");
console2.log(" ", mockUSDCToken.allowance(address(capitalPool), address(tokenManager)));
​
console2.log("They Both Match!");
}

the last Assertion checks TokenManager Contract Allowance Amount and compares it to type(uint256).max.

You can Run the Test with Following Command:

forge test --match-test test_anyoneCanCallApproveFunction -vvvv

Take a Look at the Logs:

Logs:
This is type(uint256)max:
115792089237316195423570985008687907853269984665640564039457584007913129639935
This is The Allowance Amount the 'CapitalPool' Contract Gaved to `TokenContract`:
115792089237316195423570985008687907853269984665640564039457584007913129639935
They Both Match!

Recommended Mitigation

Implement access control to restrict the approve() function to authorized callers, such as the TokenManager:

+ function approve(address tokenAddr) external onlyTokenManager {
address tokenManager = tadleFactory.relatedContracts(RelatedContractLibraries.TOKEN_MANAGER);
​
(bool success, ) = tokenAddr.call(abi.encodeWithSelector(APPROVE_SELECTOR, tokenManager, type(uint256).max));
​
if (!success) {
revert ApproveFailed();
}
}
​
+ modifier onlyTokenManager() {
+ require(msg.sender == tokenManager, "Caller is not the token manager");
+ _;
+ }

This ensures only the TokenManager can call the approve() function, preventing unauthorized access.

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.