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

Unvalidated Token Address in CapitalPool Contract Allows Malicious Contract Interactions

juggernaut63

Summary

The approve function in the CapitalPool contract lacks proper input validation, allowing any address, including invalid or malicious contracts, to be approved. This can lead to unexpected behavior and potential security risks.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L24-L39

The approve function does not validate the tokenAddr parameter, allowing any address to be passed. This can be exploited by an attacker to cause the contract to interact with unintended or malicious contracts.

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.13;
​
import "forge-std/Test.sol";
import "../src/core/CapitalPool.sol";
import {ICapitalPool} from "../src/interfaces/ICapitalPool.sol";
import {RelatedContractLibraries} from "../src/libraries/RelatedContractLibraries.sol";
import {Rescuable} from "../src/utils/Rescuable.sol";
import {CapitalPoolStorage} from "../src/storage/CapitalPoolStorage.sol";
import {ERC20} from "../lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract MockToken is ERC20 {
constructor() ERC20("Mock Token", "MTKN") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
​
contract MockTadleFactory {
address public tokenManager;
​
constructor(address _tokenManager) {
tokenManager = _tokenManager;
}
​
function relatedContracts(bytes32) external view returns (address) {
return tokenManager;
}
}
​
contract CapitalPoolTest is Test {
CapitalPool capitalPool;
MockToken tokenPenyerang;
MockTadleFactory tadleFactory;
address penyerang = address(0x1234);
address tokenManager = address(0x5678);
​
function setUp() public {
tadleFactory = new MockTadleFactory(tokenManager);
capitalPool = new CapitalPool();
tokenPenyerang = new MockToken();
​
// Set the tadleFactory in the CapitalPool contract
vm.etch(address(capitalPool), address(tadleFactory).code);
}
​
function testNoInputValidation() public {
// Attacker calls approve with invalid address
vm.prank(penyerang);
vm.expectRevert();
capitalPool.approve(address(0));
}
}

forge test --match-path test/CapitalPoolTest.t.sol
[⠊] Compiling...
[⠰] Compiling 1 files with Solc 0.8.26
[⠔] Solc 0.8.26 finished in 1.30s
Compiler run successful!

Ran 1 test for test/CapitalPoolTest.t.sol:CapitalPoolTest
[PASS] testNoInputValidation() (gas: 10627)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 578.00µs (75.00µs CPU time)

Ran 1 test suite in 5.42ms (578.00µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

  • The contract may interact with unintended addresses, leading to unexpected behavior.

  • If a malicious contract is approved, it could exploit the lack of validation to perform unauthorized actions.

  • Invalid addresses can cause transaction failures, potentially leading to denial of service.

Tools Used

  • Manual review

  • Foundry

Recommendations

  • Make sure tokenAddr is a valid contract address before proceeding with approval.

function approve(address tokenAddr) external onlyTokenManager {
require(tokenAddr != address(0), "Invalid token address");
require(isContract(tokenAddr), "Address is not a contract");
​
address tokenManager = tadleFactory.relatedContracts(RelatedContractLibraries.TOKEN_MANAGER);
(bool success, ) = tokenAddr.call(
abi.encodeWithSelector(
APPROVE_SELECTOR,
tokenManager,
type(uint256).max
)
);
​
if (!success) {
revert ApproveFailed();
}
}
​
function isContract(address addr) internal view returns (bool) {
uint256 size;
assembly {
size := extcodesize(addr)
}
return size > 0;
}

  • Add access control to ensure only tokenManager can call the approve function.

modifier onlyTokenManager() {
require(msg.sender == tadleFactory.relatedContracts(RelatedContractLibraries.TOKEN_MANAGER), "Not token manager");
_;
}
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.