code-423n4 / 2021-04-marginswap-findings

1 stars 0 forks source link

Role 9 in Roles.sol #10

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Email address

mail@gpersoon.com

Handle

gpersoon

Eth address

gpersoon.eth

Vulnerability details

This is a minor suggestion.

Roles.sol contains the following: roles[msg.sender][9] = true; It's not clear what the number 9 means. In RoleAware.sol there is a constant with the value 9: uint256 constant TOKEN_ACTIVATOR = 9;

Impact

The code is more difficult to read without an explanation for the number 9. In case the code would be refactored in the future and the constants in RoleAware.sol are renumbered, the value in Roles.sol would no longer correspond to the right value.

Recommended mitigation steps

Move the constants from Roles.sol to RoleAware.sol and replace 9 with the appropriate constant.

werg commented 3 years ago

Yes! I recently saw this as well and already fixed it in our code base 😉