Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checks for enum bounds #9

Open
code423n4 opened this issue Jun 13, 2021 · 1 comment
Open

Checks for enum bounds #9

code423n4 opened this issue Jun 13, 2021 · 1 comment

Comments

@code423n4
Copy link
Contributor

Handle

gpersoon

Vulnerability details

Impact

For the enums Mode and State, checks are made that the variables are within bounds. Here specific size are used, e.g. 2 and 4.
If the size of the enums would be changed in the future, those numbers don't change automatically.
Also solidity provides in-built check to check that variables are within bounds, which could be used instead. This also make the code more readable.

Proof of Concept

// https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/RCMarket.sol#L202
enum Mode {CLASSIC, WINNER_TAKES_ALL, SAFE_MODE}
function initialize(
...
assert(_mode <= 2); // can be removed
...
mode = Mode(_mode); // this makes sure: 0<=mode<=2 // move to top

https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/interfaces/IRCMarket.sol#L7
enum States {CLOSED, OPEN, LOCKED, WITHDRAW}

// https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/RCMarket.sol#L1094
function _incrementState() internal {
assert(uint256(state) < 4); // can be removed
state = States(uint256(state) + (1)); // this makes sure: 0<=state<=3
emit LogStateChange(uint256(state));
}

Tools Used

Recommended Mitigation Steps

For function initialize:
Remove the "assert(_mode <= 2);" and move the statement "mode = Mode(_mode);" to the top of the function and add a comment

For function _incrementState:
Remove "assert(uint256(state) < 4);" and add a comment at "state = States(uint256(state) + (1));"

@Splidge
Copy link
Collaborator

Splidge commented Jun 21, 2021

Fixed here
With some additional changes to the Enums as mentioned in #162 , implemented here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants