code-423n4 / 2024-08-phi-findings

1 stars 1 forks source link

anyone can update _credIdsPerAddress by calling _addCredIdPerAddress or _removeCredIdPerAddress #106

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/Cred.sol#L685-L729

Vulnerability details

Impact

The functions _addCredIdPerAddress and _removeCredIdPerAddress are declared as public, allowing any external user to call them and modify the _credIdsPerAddress mapping. This can lead to significant security issues, such as unauthorized users adding or removing credentials associated with another user's address. This could cause a loss of important credential data for users or allow malicious actors to manipulate the data for personal gain, potentially compromising the integrity of the system.

Specifically:

Unauthorized Manipulation: Malicious users can call _addCredIdPerAddress to add credentials to another user's address, potentially flooding their account with unwanted or fraudulent credentials. Data Tampering: Malicious users can call _removeCredIdPerAddress to remove credentials from another user's address, leading to potential data loss or denial of service. Integrity Risks: The ability to manipulate these mappings could disrupt the system's integrity, allowing attackers to create discrepancies between the actual ownership and the recorded data in the system.

Proof of Concept

Example Scenario:

Unauthorized Addition: An attacker calls _addCredIdPerAddress(12345, victimAddress) to add the credential 12345 to the victim's address. This could cause the victim's address to be associated with incorrect or malicious credentials.

Unauthorized Removal: An attacker calls _removeCredIdPerAddress(12345, victimAddress) to remove the credential 12345 from the victim's address. This could cause the victim to lose access to a credential they legitimately own.

Exploit Script:

solidity

// Assume attacker has access to these functions.

contract Exploit {
function exploitAdd(address target) public {
    _addCredIdPerAddress(12345, target); // Adds credential 12345 to the target's address
}

function exploitRemove(address target) public {
    _removeCredIdPerAddress(12345, target); // Removes credential 12345 from the target's address
}

}

Logs/Proof:

Logs from calling _addCredIdPerAddress and _removeCredIdPerAddress show that an unauthorized user successfully modifies another user’s credentials.

Tools Used

Solidity Compiler
Remix IDE or Foundry for testing and simulating the exploit.
Etherscan or Hardhat for transaction tracing and log analysis.

Recommended Mitigation Steps

Access Control: Implement proper access control mechanisms such as onlyOwner, onlyAuthorized, or role-based access using OpenZeppelin’s AccessControl to restrict who can call these functions. Example:

solidity

    function _addCredIdPerAddress(uint256 credId_, address sender_) public onlyAuthorized {
        ...
    }

    function _removeCredIdPerAddress(uint256 credId_, address sender_) public onlyAuthorized {
        ...
    }

Event Logging: Ensure that events are emitted whenever a credential is added or removed, to provide an audit trail and help detect unauthorized actions.

Input Validation: Implement checks to ensure that the caller is allowed to modify the credentials for the specified address, possibly by verifying ownership or using an authorization system.

Assessed type

Other

c4-judge commented 1 month ago

fatherGoose1 marked the issue as satisfactory