Carrieverse - audit

In the function `EmployeeTokenLock.withdraw()`, `lockupInfo[_lockupIndex]` is deleted after transferring the tokens, which violates the check-effect-interaction pattern. If the token has a hook, it can reenter the same function and drain token balance. Functions `EmployeeTokenLock.withdraws()`, `TokenLock.withdraw()`, and `TokenLock.withdraws()` share the same issue. ```solidity=249 function withdraw(uint256 _lockupIndex) public onlyLock(_lockupIndex) returns(uint256 withdrawBalance){ uint256 balance = getBalance(_lockupIndex); require(balance > 0, "EmployeeTokenLock: NOT_AMOUNT"); require(IERC20(token).transferFrom(address(this), msg.sender, balance)); delete lockupInfo[_lockupIndex]; uint256 index = 0; for( index; index < accountToLockupIndex[msg.sender].length; index++ ){ if( accountToLockupIndex[msg.sender][index] == _lockupIndex ){ break; } } if( accountToLockupIndex[msg.sender].length > 0 ){ accountToLockupIndex[msg.sender][index] = accountToLockupIndex[msg.sender][accountToLockupIndex[msg.sender].length-1]; accountToLockupIndex[msg.sender].pop(); } totalAmount -= balance; lockupCount--; withdrawBalance = balance; emit withdrawEvent(msg.sender, _lockupIndex, balance); } ```

In contract `TokenLock`, the `setLockup()` function checks for address, amount and duration as following, but the `setLockups()` function does not contain these checks. ```solidity=87 require( _account != address(0), "TokenLock: NOT_ADDRESS"); require( _amount > 0, "TokenLock: NOT_AMOUNT"); require( _duration > 0, "TokenLock: NOT_DURATION"); ``` This also applies to contract `EmployeeTokenLock`.

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

The following addresses should be checked before assignment to make sure they are not zero addresses. In contract `EmployeeTokenLock`: ```solidity=49 admin = _admin; token = _token; ``` In contract `TokenLock`: ```solidity=44 admin = _admin; token = _token; ```

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.