Skip to content

Instantly share code, notes, and snippets.

@danbogd
Last active August 20, 2019 15:11
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save danbogd/83c8a366c73cc15ce1f46f71bc97cb0f to your computer and use it in GitHub Desktop.
Save danbogd/83c8a366c73cc15ce1f46f71bc97cb0f to your computer and use it in GitHub Desktop.

LCX audit report.

1. Summary

This document is a security audit report performed by danbogd, where LCX has been reviewed.

2. In scope

Сommit hash .

3. Findings

In total, 4 issues were reported including:

  • 1 medium severity issues
  • 1 low severity issues
  • 2 owner privileges (ability of owner to manipulate contract, may be risky for investors).
  • 0 notes.

No critical security issues were found.

3.1. No checking for zero address

Severity: low

Description

Incoming addresses should be checked for an empty value(0x0 address).

Code snippet

Lines 241, 561.

        function setTokenAddress(IERC20 token) public onlyOwner returns(bool){
        LCXToken = token;
        return true;
        }


        function setTokenVestingAddress(TokenVesting tokenVestingAddress) public onlyOwner returns(bool){
        vestingContractAddress = tokenVestingAddress;
        return true;
        }       

3.2. Owner Privileges

Severity: owner previliges

Description

Contract owner allow himself to:

  • upgrade contract and implement any logic in the new contract. And even if the new contract will be audited, at any time possible to change the address of the new contract again to not audited and insecure.

Lines 241, 561.

        function setTokenAddress(IERC20 token) public onlyOwner returns(bool){
        LCXToken = token;
        return true;
        }


        function setTokenVestingAddress(TokenVesting tokenVestingAddress) public onlyOwner returns(bool){
        vestingContractAddress = tokenVestingAddress;
        return true;
        }       
  • revoke the vesting.
        function revoke(address account) public onlyOwner {
        VestedToken storage vested = vestedUser[account];
        require(!vested.revoked);
        uint256 balance = vested.totalToken;
        uint256 unreleased = _releasableAmount(account);
        uint256 refund = balance.sub(unreleased);
        vested.revoked = true;
        vested.totalToken = unreleased;
        LCXToken.safeTransfer(owner(), refund);
        emit VestingRevoked(account);
        }  

3.3. Bypassing Smart Contract Timelocks

Severity: medium

Description

There are no checks that the beneficiary address can be a contract address.

Example:

The user was able to divest himself of his interest even though the tokens never moved. He didn’t sell the timelocked tokens itself. He sold the future ownership of the tokens. The user is asked by the deployer of the LCX vesting contract for the address where he’d like to receive his tokens after the releaseTime expires in 3 years. User deploys the “Bypasser” contract and gives its address to the deployer of the LCX vesting contract. The Bypasser contract didn’t magically make the timelocked tokens transferable — it made the future ownership of the timelocked tokens transferable.

More details here.

Code snippet

Line: 262.

        function _setVesting(address account, uint256 amount, uint256 cliff, uint256 duration, uint256 startAt) internal {
         
         require(account!=address(0));
         require(cliff<=duration);
         VestedToken storage vested = vestedUser[account];
         vested.cliff = cliff;
         vested.start = startAt;
         vested.duration = duration;
         vested.totalToken = amount;
         vested.releasedToken = 0;
         vested.revoked = false;
         }

4. Conclusion

The review did not show any critical issues, some of medium and low severity issues were found.

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