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

Everyone can call `approve` function

canbooo

Summary

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

In CapitalPool.sol , function approve must only can be called by token manager but there is no limitation for that. Everyone can call this function.

// 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() {}
​
/**
* @dev Approve token for token manager
* @notice only can be called by token manager
* @param tokenAddr address of token
*/
// @audit everyone can call approve function?
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();
}
}
}
​

Vulnerability Details

Line link: https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L233

In TokenManager.sol there is a function which is function _transfer and uses this approve to send tokens.

function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
​
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
ICapitalPool(_capitalPoolAddr).approve(address(this));
}
​
_safe_transfer_from(_token, _from, _to, _amount);
​
uint256 fromBalanceAft = IERC20(_token).balanceOf(_from);
uint256 toBalanceAft = IERC20(_token).balanceOf(_to);
​
if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
​
if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}
// @audit burada toBalanceAft'ye başka yerden aynı esnada transfer
// @audit yapılırsa bakiye daha fazla artacağı için eşitlik sağlanmamış
// @audit ve revert etmiş olmaz mı haksız yere?
}

Impact

The fact that the approve function can be called by anyone, especially when used together with the _transfer function, can lead to serious security vulnerabilities. These vulnerabilities could allow malicious individuals to exploit the system, resulting in unjust gains or disrupting the proper functioning of the system.

Potential System Vulnerabilities

Unauthorized Transfer Authorization:

The unrestricted access to the approve function allows unauthorized users to perform approval actions on the CapitalPool contract.
This could lead to unlimited allowances being granted to the tokenManager address, allowing it to withdraw any desired tokens from CapitalPool.
In such a case, a malicious actor could use the approve function to transfer tokens within the system to another address.

Manipulation and Double-Spending Risk:

A malicious user could continuously call the approve function, frequently granting unlimited allowances to the tokenManager address.
With these allowances, it could be possible to repeatedly transfer the same tokens, enabling double-spending or manipulation.

Denial of Service (DoS) Attacks:

A malicious actor could repeatedly call the approve function to overload the system, causing it to become busy.
This could consume the system's resources, leading to slowdowns or even a complete halt in processing."

Tools Used

Manual review

Recommendations

An access control like the following can be added at the beginning of the function:

// 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() {
+ require(msg.sender == tokenManager, "Caller is not the token manager");
+ _;
+ }
​
/**
* @dev Approve token for token manager
* @notice only can be called by token manager
* @param tokenAddr address of token
*/
​
+ 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();
}
}
}
​
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.