code-423n4 / 2024-01-curves-findings

0 stars 0 forks source link

Missing require() #663

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Security.sol#L8-L16

Vulnerability details

Impact

If require statements are not used in Security.sol, the purpose of the modifiers cannot be faithfully executed. Modifiers are intended to halt the execution of a function and revert the transaction if specific conditions are not met. Therefore, not using require can lead to the following consequences:

  1. onlyOwner: If msg.sender is not the owner, the function continues to execute, allowing entities other than the owner to use owner privileges. This means critical parts of the contract can be manipulated by entities other than the owner.

  2. onlyManager: If msg.sender is not a manager, the function continues to execute, enabling entities other than managers to use manager privileges. This implies that operations requiring manager privileges could be performed indiscriminately.

  3. Access control in other functions: Several functions use the onlyOwner or onlyManager modifiers to allow only specific entities to call them. Not using require would render such access control ineffective, allowing unauthorized entities to call functions requiring certain permissions.

In essence, not using require in Security.sol would compromise the proper functioning of functions requiring permissions, creating security vulnerabilities where unexpected entities can manipulate the contract.

Proof of Concept

Sorry, this is my first DeFi BugBounty report, so I couldn't prepare the PoC. However, I think the problem is clear enough to know just by analyzing the code. In the future, I will study more about DeFi vulnerabilities and DeFi Bugbounty so that I can report good quality.

Tools Used

N/A

Recommended Mitigation Steps

modifier onlyOwner() {
    require(msg.sender == owner, "Not the owner");
    _;
}

modifier onlyManager() {
    require(managers[msg.sender], "Not a manager");
    _;
}

Assessed type

Access Control

c4-pre-sort commented 7 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #3

c4-judge commented 7 months ago

alcueca marked the issue as unsatisfactory: Invalid