ETH_AdChain_audit_report.md

AdChain audit report.

1. Summary

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

2. In scope

Сommit hash 5930cede2d533c8ee92001fd0be8eada2f2bc614.

• Disbursement.sol.
• Filter.sol.
• HumanStandardToken.sol.
• Migrations.sol.
• Sale.sol.
• StandardToken.sol.
• Token.sol.
• AttributeStore.sol.
• Challenge.sol.
• DLL.sol.
• Migrations.sol.
• PLCRVoting.sol.
• Parameterizer.sol.
• Registry.sol.

3. Findings

In total, 11 issues were reported including:

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

No critical security issues were found.

3.1 Non-compliance with ERC-20 Token Standard.

Severity: medium

Description

ERC-20 Token Standard specifies for functions transfer and transferFrom:

The function SHOULD throw if the _from account balance does not have enough tokens to spend. But in this implementation it just returns false. This can lead to serious consequences. Because checking the return value of this function is rare.

Code snippet

https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/StandardToken.sol#L15 https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/StandardToken.sol#L28

3.2. 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. 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.3. The relocating is not secure process for investors.

Severity: note

Description

The owners can 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.

Code snippet

https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Disbursement.sol#L67 https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Filter.sol#L37

3.4. Multi Transfer arrays length check.

Severity: low

Description

There are two/three input arrays, but no check for the same length. In this case it is possible to skip some parameters.

Code snippet

https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Filter.sol#L22-L35 https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Sale.sol#L120 https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Sale.sol#L99

3.5. Owner Privileges

Severity: owner previliges

Description

The contract owner allow himself to:

• changeOwner;
• changePrice;
• changeWallet;
• changeStartBlock at which contract will begin selling;
• emergency stop sales.

3.6. Missing event call.

Severity: low

Description

According to ERC20 standard, when initializing a token contract if any token value is set to any given address a transfer event should be emitted. An event isn't emitted when assigning the initial supply to the msg.sender.

###Code snippet

https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/HumanStandardToken.sol#L44 https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/Sale.sol#L92

3.7. Deprecated method.

Severity: note

Description

Implementation of fallback function is redundant, because when funds are transferred to the contract without this function, exception will also be thrown.

Code snippet

https://github.com/AdChain/AdChainRegistry/blob/5930cede2d533c8ee92001fd0be8eada2f2bc614/contracts/historical/HumanStandardToken.sol#L20

4. Conclusion

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