Skip to content

Instantly share code, notes, and snippets.

@yuriy77k
Forked from MrCrambo/ETH_SelfKey_report.md
Created June 16, 2019 17:04
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save yuriy77k/185cdb3506cbf02c027ee24cb874ffd6 to your computer and use it in GitHub Desktop.
Save yuriy77k/185cdb3506cbf02c027ee24cb874ffd6 to your computer and use it in GitHub Desktop.

Summary

This is the report from a security audit performed on SelfKey by MrCrambo.

The audit focused primarily on the security of SelfKey smart contracts.

In scope

  1. https://github.com/SelfKeyFoundation/selfkey-main-contracts/blob/develop/contracts/PaymentSplitter.sol
  2. https://github.com/SelfKeyFoundation/selfkey-main-contracts/blob/develop/contracts/SelfKeyMain.sol

Findings

In total, 3 issues were reported including:

  • 0 high severity issues.

  • 1 medium severity issues.

  • 0 owner privilegies issues.

  • 2 low severity issues.

Security issues

1. Zero address checking

Severity: low

Description

There are no zero address checking in functions makePayment and setAddress.

2. Making payment error

Severity: medium

Description

In function makePayment if affiliate1Address is not zero address and affiliate2Address is zero address then affiliate1Address will receive tokens and affiliate2Address will not receive it. But if affiliate1Address is zero address and affiliate2Address is not zero address then both of them will not receive, but affiliate2Address should receive it.

Recommendation

Checking affiliate2Address for zero address should be outside of if statement which checks affiliate1Address for zero address.

3. Known vulnerabilities of ERC-20 token

Severity: low

Description

  1. It is possible to double withdrawal attack. More details here
  2. Lack of transaction handling mechanism issue. More details here

Recommendation

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

require( _to != address(this) );

Conclusion

Smart contract contains medium severity issue.

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