Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: low
Invalid

Allowance verification in CapitalPool contract

Summary

The allowance check for the old and new token manager are not correctly validating the allowance.

Vulnerability Details

The issue lies in the verification of allowances when the approve function is called in the CapitalPool contract. Specifically, when the token manager address changes, the contract might not correctly revoke or update the old allowances, leading to a scenario where both the old and new token managers have valid allowances to spend the same tokens.

Impact

If the old token manager's allowance is not correctly revoked, this could lead to unauthorized token transfers by the old manager. This could be exploited if the old token manager's address is compromised or malicious.

I have created a simple POC with CapitalPoolEdgeTest.t.sol and added mocktokenmanger in mocks and utilized MockERC20Token.

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.13;
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockERC20Token is ERC20 {
constructor() ERC20("MockToken", "MT") {}
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
contract MockTokenManager {
// This contract can have more complex logic if needed, but for now it serves as a placeholder
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "../src/core/CapitalPool.sol";
import "../test/mocks/MockERC20Token.sol";
import "../test/mocks/MockTokenManager.sol";
contract CapitalPoolEdgeTest is Test {
CapitalPool public capitalPool;
MockToken public token;
address public tokenManager;
function setUp() public {
token = new MockToken();
tokenManager = address(new MockTokenManager());
capitalPool = new CapitalPool();
// Assuming tadleFactory and relatedContracts are properly mocked/set up
// Set tokenManager address in tadleFactory.relatedContracts
}
function testTokenManagerAddressChange() public {
vm.startPrank(tokenManager);
capitalPool.approve(address(token));
address newTokenManager = address(0x789);
// Update relatedContracts to return newTokenManager for TOKEN_MANAGER
// Assuming tadleFactory is set up to change the token manager
capitalPool.approve(address(token));
uint256 newAllowance = token.allowance(address(capitalPool), newTokenManager);
assertEq(newAllowance, type(uint256).max);
uint256 oldAllowance = token.allowance(address(capitalPool), tokenManager);
assertEq(oldAllowance, 0); // Old manager should no longer have allowance
}
}

Output

Tools Used

Foundry

Recommendations

Consider implementing a more secure pattern where approvals are time-limited or require explicit revocation before new approvals are granted.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-Admin-Errors-Malicious

The following issues and its duplicates are invalid as admin errors/input validation/malicious intents are1 generally considered invalid based on [codehawks guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). If they deploy/set inputs of the contracts appropriately, there will be no issue. Additionally admins are trusted as noted in READ.ME they can break certain assumption of the code based on their actions, and

Support

FAQs

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