Skip to content

Instantly share code, notes, and snippets.

@gorbunovperm
Last active August 25, 2019 09:25
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 gorbunovperm/2e1ede34819e324ca1385cc316e21889 to your computer and use it in GitHub Desktop.
Save gorbunovperm/2e1ede34819e324ca1385cc316e21889 to your computer and use it in GitHub Desktop.
LCX security audit report

LCX security audit report

Summary

This is the report from a security audit performed on LCX by gorbunovperm.

Smart Contract Audit for Listing on Exchanges

https://www.lcx.com/

In scope

Commit hash: 2274bf5224168da72285399fca9be14471ea7764

  1. LCX

Findings

In total, 5 issues were reported including:

  • 0 critical severity issue.

  • 2 high severity issue.

  • 0 medium severity issues.

  • 2 low severity issues.

  • 1 owner privileges.

  • 0 note.

Security issues

3.1. Known vulnerabilities of ERC-20 token

Severity: low

Description

  • Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here

Recommendation

Add into a function transfer(address _to, ... ) following code:

require( _to != address(this) );

3.2. During revoking the owner receives more tokens than it should

Severity: high

Code snippet

  • Line 372.

Description

During revoke() the owner receives vested.totalToken value minus the unreleased tokens:

uint256 refund = balance.sub(unreleased);

This is wrong because it does not take into account the number of tokens already paid(vested.releasedToken).

Let's look at an example.

Passed periods totalToken releasedToken unreleasedToken TokenVesting Balance Owner Balance
0 1000 0 0 2000 0

The total number of tokens Michael should receive is 1000 during 10 vesting periods. After 2 vesting periods he calls the releaseToken function and receive 300 tokens.

Passed periods totalToken releasedToken unreleasedToken TokenVesting Balance Owner Balance
2 1000 200 0 1800 0

Then 4 more periods passed.

Passed periods totalToken releasedToken unreleasedToken TokenVesting Balance Owner Balance
6 1000 200 400 1800 0

The owner decided to cancel the vesting payout and called revoke function. He receives totalToken value minus the unreleased tokens 1000 - 400 = 600:

Passed periods totalToken releasedToken unreleasedToken TokenVesting Balance Owner Balance
6 400 200 400 1200 600

And if Michael asks for the unreleased amount, the balance of the vesting contract will be less than expected: 1200 - 400 = 800. But the initial balance was 2000 and 1000 were allocated for Michael.

3.3. The beneficiary will not receive tokens after revoking

Severity: high

Code snippet

  • Line 374
  • Line 341

Description

After the owner calls revoke function, the value of vested.totalToken variable is set to number of unreleased tokens.

    vested.totalToken = unreleased;

And when beneficiary calls releaseToken function, the amount of unreleased tokens will be calculated: _vestedAmount(account).sub(vestedUser[account].releasedToken). But _vestedAmount for revoked beneficiary is totalToken amount. In the example from issue 3.2. if in second step we passed not 4 but 1:

Passed periods totalToken releasedToken unreleasedToken TokenVesting Balance Owner Balance revoked
6 100 200 100 900 900 true

And during the releaseToken the calculation will be following: 100 - 200 = -100 and that would lead to throw.

3.4. Checking input addresses

Severity: low

Code snippet

  • Line 600.

Description

Incoming addresses should be checked for an empty value(0x0 address) to avoid loss of funds or blocking some functionality.

3.5. Owner can change token address

Severity: owner privileges

Code snippet

  • Line 241.

Description

For any reason, the owner can change the address of the token contract at any time, which may lead to the inability to receive vested tokens by beneficiary.

There is also no guarantee that the vesting contract will have enough tokens for payment.

Conclusion

There are some dangerous vulnerabilities were discovered here.

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