Missing access control on critical functions

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/MaxHeap.sol#L136

Vulnerability details

The broad admin role enables arbitrary manipulation of the heap without restrictions.

Recommendation: Implement granular access control and privilege separation. Implement an access control system such as OpenZeppelin AccessControl to restrict access to these functions. For example, create separate admin roles for each function and assign them granular privileges.

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/MaxHeap.sol#L136

The insert, updateValue, and extractMax functions in the MaxHeap contract allow the admin to insert, update, and extract values from the heap. However, there is no access control on these critical functions besides a basic onlyAdmin modifier. This allows the admin account to arbitrarily modify the heap without restrictions.

- modifier onlyAdmin() {
+ using AccessControl for AccessControl.AccessControl; 

+ bytes32 public constant INSERT_ROLE = keccak256("INSERT_ROLE");
+ bytes32 public constant UPDATE_ROLE = keccak256("UPDATE_ROLE");
+ bytes32 public constant EXTRACT_ROLE = keccak256("EXTRACT_ROLE");

+ modifier onlyInserter() {
+   require(hasRole(INSERT_ROLE, msg.sender), "Sender missing INSERT_ROLE");
   _;  
 }

+ function insert(uint256 itemId, uint256 value) public onlyInserter {
+   // ...
+ }

+ function updateValue(uint256 itemId, uint256 newValue) public onlyHolder(UPDATE_ROLE) {
+   // ...  
+ }

+ function extractMax() public onlyHolder(EXTRACT_ROLE) returns (uint256, uint256) {
+   // ...
+ }

Assessed type

Access Control

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #686

[raymondfam]

Intended design.

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Overinflated severity