0brxce - Initialize Incorrect Access Control

[sherlock-admin]

0brxce

high

Initialize Incorrect Access Control

Summary

Initialize can be called by all not just the factory contract due to lack of CEI in initialize() function. This leads to several other vulnerabilities detailed below.

Vulnerability Detail

initialize() is intended to be called only by VestingEscrowFactory and is done so in the deployVestingContract. However in VestingEscrow.sol, initialize() is an external function with no access control. initialize() is supposed to be protected by the if statement (msg.sender != factory) but this can be bypassed by supplying msg.sender as factor in _initialDelegateParams. This is because _factory is set by initialize before the if statement is evaluated. As a result you can initialize multiple times and modify the various parameters to create vulnerabilities and elicit unexpected behavior

Impact

As a result of calling initialize() you can: End vesting early and claim early, by setting lockedTime to 0. Change revokable status.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L92C5-L116C6

Tool used

Manual Review

Recommendation

Follow CEI, Implement Access Modifier for initialize().

[nevillehuang]

Invalid, check performed here.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

pratraut commented:

'invalid due to initialize will get call during deployment time'