Lines of code
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L30-L34
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L45-L50
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L21
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L39
Vulnerability details
Impact
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L30-L34
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L45-L50
msg.sender == admin,
"only the admin may call _becomeImplementation"
);
&
require(
msg.sender == admin,
"only the admin may call _resignImplementation"
);
These lines of code in the _becomeImplementation and _resignImplementation functions are checking that the msg.sender (the address of the account that called the function) is equal to the admin variable. If it is not, then the require statement will cause the function to revert and the call will fail. This means that only the address that is stored in the admin variable can call these functions, which could make the contract unable to function if the admin's address is compromised or the admin is unable to perform their duties.
If the admin's address is compromised, an attacker could take control of the contract and use it to steal assets, manipulate data, or launch further attacks.
If the admin is unable to perform their duties, the contract will be unable to function, which could result in a loss of assets or the inability to perform critical functions.
In case of a crisis, the contract would be unable to be stopped, which could create an emergency where the contract's assets could be at risk.
This vulnerability could also impact the contract's scalability and usability, as there is no contingency plan in case the admin's address is compromised or the admin is unable to perform their duties.
Proof of Concept
The attacker can obtain the address of the admin: This can be done by calling the admin() function on the contract or by inspecting the contract's code.
An attacker can compromise the admin's address. This can be done by various means such as phishing, social engineering, or other hacking methods that could lead to the attacker gaining control over the admin's address.
Call the _becomeImplementation or _resignImplementation functions: Once the attacker has the admin's address, they can call the _becomeImplementation or _resignImplementation functions, which should only be able to be called by the admin.
The attacker could also take control of the contract by calling the _becomeImplementation function and potentially steal assets or manipulate data.
Here is an example.
// Step 1: Obtain the admin address
address admin = CCashDelegate.admin();
// Step 2: Compromise the admin's address (not shown)
// Step 3: Call the _resignImplementation function
CCashDelegate.resignImplementation();
// Step 4: Observe the impact
// The contract will be unable to function as the _resignImplementation function can only be called by the admin
Tools Used
Manual audit, Vs Code.
Recommended Mitigation Steps
The first step is to Implement a multi-signature mechanism. Allow multiple addresses to have administrative capabilities on the contract. This can be done by implementing a multi-signature mechanism that requires a certain number of signatures to perform certain actions.
Implement an emergency stop mechanism. Allow the contract to be halted in case of a critical security vulnerability or other emergencies. This can be done by implementing a mechanism that allows a specific address or group of addresses to halt the contract's functionality.
Implement a mechanism to change the admin address. Allow the admin address to be changed in case the current one is compromised or unable to perform its duties.
Regularly review and update the contract to ensure that it is secure and functional.
Code snippet for a multi-signature mechanism.
// Declare an array of addresses for the admin
address[] adminAddresses;
// Update the admin function to add the new admin
function addAdmin(address _newAdmin) public {
require(msg.sender == adminAddresses[0], "Only the current admin can add a new admin.");
adminAddresses.push(_newAdmin);
}
// Update the _becomeImplementation function to require multiple signatures
function _becomeImplementation(bytes memory data) public virtual override {
// Shh -- currently unused
data;
// Shh -- we don't ever want this hook to be marked pure
if (false) {
implementation = address(0);
}
require(
address(this).hasAuthority(msg.sender, adminAddresses),
"only adminAddresses may call _becomeImplementation"
);
}
Impact
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L21
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCashDelegate.sol#L39
function _becomeImplementation(bytes memory data) public virtual override {
function _resignImplementation() public virtual override {
This is caused by the fact that the _becomeImplementation and _resignImplementation functions are defined as "public" which means that anyone can call these functions if they have the contract's address, regardless of whether or not they have the necessary permissions to do so. There is no mechanism in place to restrict the functions to specific addresses or roles, which could allow unauthorized users to access sensitive data or manipulate the contract's state.
An attacker could call the _becomeImplementation or _resignImplementation functions and take control of the contract, stealing assets or manipulating data.
An attacker could access sensitive information stored in the contract, such as private keys or user data, by calling functions that are intended to be private.
An unscrupulous attacker could create a malicious contract that calls the functions in the contract with unintended parameters, causing unexpected behavior.
A hacker could cause a denial-of-service attack by repeatedly calling the functions, causing the contract to run out of gas or become unresponsive.
It could also impact the contract's scalability and usability, as any address can call the functions without any restriction.
Proof of Concept
-
Obtain the address of the contract This can be done by deploying the contract to a testnet or by inspecting the contract's code.
-
Call the _becomeImplementation or _resignImplementation functions: With the contract address, anyone can call the _becomeImplementation or _resignImplementation functions.
-
Observe the impact of the vulnerability as a result of the attacker's actions, the contract should be manipulated by the attacker, and sensitive data could be exposed or stolen.
The following is an example of an attacker who could use to exploit the vulnerability.
// Step 1: Obtain the contract address
address contractAddress = "0x1234....";
// Step 2: Create a contract instance
CCashDelegate contract = CCashDelegate(contractAddress);
// Step 3: Call the _resignImplementation function
contract.resignImplementation();
// Step 4: Observe the impact
// The contract can be manipulated by the attacker, and sensitive data could be exposed or stolen
Tools Used
Manual audit, vs code
Recommended Mitigation Steps
To mitigate this Vulnerability below is a recommended step.
-
Implement role-based access control to restrict the functions to specific addresses or roles by using an access control library such as OpenZeppelin's AccessControl This will ensure that only authorized users can call the functions.
-
Use the internal visibility keyword to change the visibility of the functions to "internal" so that only contracts that inherit from the contract can call the functions.
-
Use the "private" visibility keyword to change the visibility of the functions to "private" so that only the contract's code can call the functions.
-
Implement a circuit breaker mechanism Implement a mechanism that allows a specific address or group of addresses to halt the contract's functionality in case of an emergency.
import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/access/Roles.sol";
contract CCashDelegate is CCash, CDelegateInterface, Roles {
// Declare a role for the admin
using Roles for Roles.Role;
Roles.Role admin;
// Update the _becomeImplementation function to require the admin role
function _becomeImplementation(bytes memory data) public virtual override {
// Shh -- currently unused
data;
// Shh -- we don't ever want this hook to be marked pure
if (false) {
implementation = address(0);
}
require(
admin.has(msg.sender),
"only the admin may call _becomeImplementation"
);
}
}
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent