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

Missing access control in ```CapitalPool::approve``` enables any address to approve unlimited amounts

kiteweb3

Summary

The CapitalPool::approve function is intended to allow only the designated token manager to approve tokens (as indicated in the natspec here https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/interfaces/ICapitalPool.sol#L11). However, the current implementation lacks proper access control mechanisms, enabling any address to call this function and approve unlimited amounts of tokens without restriction.

Link: https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L24C5-L39C6

Vulnerability Details

@> 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();
}
}

Impact

The absence of an access control mechanism, such as a modifier that verifies the caller's identity, means that the approve function does not enforce its intended restriction to the token manager only. This oversight allows unauthorized users to execute the function.

An attacker exploiting this vulnerability can approve tokens from the contract to any address, including their own, without needing authorization from the token manager and transfer the funds.

Copy and paste this test in PreMarket.t.sol

Run forge test --match-test test_notOnlyTokenManager

function test_notOnlyTokenManager() public {
vm.prank(user);
capitalPool.approve(address(mockUSDCToken));
vm.stopPrank();
}
​
Log:
⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.25
[⠘] Solc 0.8.25 finished in 2.77s
Compiler run successful!
​
Ran 1 test for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_notOnlyTokenManager() (gas: 53764)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.69ms (69.29µs CPU time)
​
Ran 1 test suite in 283.01ms (3.69ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual review

Recommendations

Implement a modifier to enforce access control for the approve function, ensuring that only the designated token manager can invoke it.

+ modifier onlyTokenManager() {
+ address tokenManager = tadleFactory.relatedContracts(
+ RelatedContractLibraries.TOKEN_MANAGER
+ );
+ require(msg.sender == tokenManager, "Caller is not the token manager");
+ _;
+ }
​
- function approve(address tokenAddr) external {
+ function approve(address tokenAddr) external onlyTokenManager {
​
... omitted code
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.