To be considered trustless, both the incentives and the code must be aligned to prevent the possibility of maliciousness, especially by actors of the system given specific powers.
Impact
Administrators are able to rug all funds in the gravity bridge, which requires a lot of trust
Proof of Concept
File: solidity/contracts/Gravity.sol #1
632 function withdrawERC20(
633 address _tokenAddress)
634 external {
635 require(cudosAccessControls.hasAdminRole(msg.sender), "Recipient is not an admin");
636 uint256 totalBalance = IERC20(_tokenAddress).balanceOf(address(this));
637 IERC20(_tokenAddress).safeTransfer(msg.sender , totalBalance);
638 }
If this function were able to withdraw to a pre-defined address, or the function required a specific amount to be specified, then under the right circumstances, maybe it would be ok, but the fact that it goes directly to the message sender, and only the full balance can be transferred, and the bridge's token can be taken, makes it seem like this is just a rug vector waiting to be applied. This amount of privilege is excessive.
Tools Used
Code inspection
Recommended Mitigation Steps
Only allow withdrawal to a multisig after a timelock has expired, and only of specific amounts. Also, consider making the bridge pausable to mitigate attacks
Lines of code
https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L632-L638
Vulnerability details
https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/README.md?plain=1#L14
To be considered trustless, both the incentives and the code must be aligned to prevent the possibility of maliciousness, especially by actors of the system given specific powers.
Impact
Administrators are able to rug all funds in the gravity bridge, which requires a lot of trust
Proof of Concept
https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L632-L638
If this function were able to withdraw to a pre-defined address, or the function required a specific amount to be specified, then under the right circumstances, maybe it would be ok, but the fact that it goes directly to the message sender, and only the full balance can be transferred, and the bridge's token can be taken, makes it seem like this is just a rug vector waiting to be applied. This amount of privilege is excessive.
Tools Used
Code inspection
Recommended Mitigation Steps
Only allow withdrawal to a multisig after a timelock has expired, and only of specific amounts. Also, consider making the bridge pausable to mitigate attacks