Privilege Escalation

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/INSTR.sol#L42

Vulnerability details

Impact

An attacker with permissions to the INSTR.store() function can obtain the "executor" and "admin" role. This implementation also gives space to human error because when the Kernel's executor changes, the permissions to INSTR.store() needs to be changed manually as well. By not changing the permission the old executor can reclaim his/her role by pushing an Instruction to the queue.

Exploitation scenario 1

• Address1 is the executor of Kernel
• The executor gives permission to Address2 to use INSTR.store()
• Address2 pushes a "ChangeExecutor" Instruction to the list
• Optionally Address2 pushes a "ChangeAdmin" Instruction as well
• Address2 now escalated his/her privileges

Exploitation scenario 2

• Address1 is the executor of the Kernel
• Address1 also already has permission to INSTR.store()
• Address1 gives the executor rank to Address2
• Address1 keeps the permission to use INSTR.store() and can push Instructions including "ChangeExecutor" and "ChangeAdmin"

Methodology used

Manual code audit

Recommended Mitigation Steps

I recommend changing the modifier of INSTR.store() so it can only be called by the Kernel's executer address.

Another mitigation option: when an executor changes, change the permission for the function as well but for this the smart contract also needs to require that only one address has access to this permission.

[fullyallocated]

This is intended behavior. Only Policies can call Modules so Address 1 would have to write a policy contract that does this. Addresses cannot call Modules directly.

[0xean]

closing as invalid.