EthereumCommonwealth / Auditing

Ethereum Commonwealth Security Department conducted over 400 security audits since 2018. Not even a single contract that we audited was hacked. You can access our audit reports in the ISSUES of this repo. We are accepting new audit requests.
https://audits.callisto.network/
GNU General Public License v3.0
132 stars 34 forks source link

Ethereum Anonymizer #7

Closed yuriy77k closed 6 years ago

yuriy77k commented 6 years ago

Audit request

Ethereum Anonymizer - service and wallet for anonymous transactions of any tokens and Ether in Ethereum network. https://satoshi.team/

Source code

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d

Disclosure policy

Publish everything.

Platform

ETH

RideSolo commented 6 years ago

Auditing time: 7 days.

yuriy77k commented 6 years ago

@RideSolo assigned.

MrCrambo commented 6 years ago

Auditing time 7 days.

yuriy77k commented 6 years ago

@MrCrambo assigned.

gorbunovperm commented 6 years ago

The estimated auditing time is 7 days.

yuriy77k commented 6 years ago

@gorbunovperm assigned.

alexo18 commented 6 years ago

Auditing time 7 days.

yuriy77k commented 6 years ago

@alexo18 assigned.

yuriy77k commented 6 years ago

EthereumAnonymizer informed about critical issues.

yuriy77k commented 6 years ago

@alexo18 @MrCrambo @gorbunovperm stop auditing this contract. I close this issue.

yuriy77k commented 6 years ago

Ethereum Anonymizer Project Audit Report.

1. Summary

This document is a security audit report performed by RideSolo, where Ethereum Anonymizer Project has been reviewed.

The aim of this project is to implement a contract who handles transaction while respecting the anonymity of the users. ATH ERC20 TOKEN uses advanced cryptographic techniques, namely zero-knowledge proofs, to guarantee the validity of transactions without revealing additional information about them.

2. In scope

2.1. Excluded

3. Findings

7 issues were reported including:

3.1. Coding Standard

Severity: minor

Description

This is a minor issue but for a better understanding of the audit report, it has to be presented first.

For code usability and understanding, the contract developers should use more intuitive names to understand the workflow of the contract.

Using seller and buyer is confusing since we expect an exchange of a defined value from both sides, however, in this contract seller is representing the receiver and the buyer is representing the sender.

The main usage of this contract is to transfer a value of ether, ATH or ERC20 compatible tokens between two addresses while keeping the transfer anonymous.

3.2. compromised anonymity

Severity: High

Description

The contract supposedly allows users to anonymously transfer ETH and any ERC-20 tokens, without revealing the connections between the sender and the recipient, but all the information are saved in the invoices state variable, invoice structure directly link seller and buyer.

When calling SELLER_STEP_1_OPEN function by the seller address and BUYER_STEP_2,BUYER_STEP_4 by the buyer address, information such as the seller address, buyer address, the generated passes and the amount are saved in the blockchain.

Please keep in mind that not setting the public specifier on state variables do not make them unreadable, even private state variable can be read directly from the blockchain (the contract developers have to remember that everything saved in the blockchain is readable).

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L272#L280 https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L290 https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L402#L424 https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L437#L466 https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L475#L492 https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L505#L543

3.3. Zero-Knowledge Proof Implementation

Severity: High

Description

Following the proposed implementation of the ZK protocol, knowing that the Helper contract was not disclosed to us, we conducted an audit over the available function related to the ZK protocol disclosed to us.

SELLER_STEP_1_OPEN function generates two passes using Helper contract, PASS1 and PASS3. PASS1 is used to map to the newly created invoice.

SELLER_STEP_2_GET_PASS return PASS1 and PASS3 to the user who called SELLER_STEP_1_OPEN (in other words the seller) without need of a transaction since the function is constant and can be called only by a single node.

Setting the specifier as constant doesn't add any extra anonymity since the passes were saved in state variables when calling SELLER_STEP_1_OPEN.

The buyer will call the constant function BUYER_STEP_1 to generate PASS2 using PASS1 who has been given to him offline buy the seller.

The buyer will therefore call BUYER_STEP_2 using PASS2 to search for PASS1 in invoicesStack, this step doesn't make any sense to the buyer since PASS1 was given to him offline. PASS1 will be used to find the invoice created previously buy the seller.

BUYER_STEP_3 is called offline by the buyer to encode the amount desired to be transferd to the seller.

BUYER_STEP_4 decode the value of the amount and set it in the previously created invoice state variable therefore making it accessible to everyone. not that the amount has to be available in the balance of the buyer inside the contract (either Ether, ERC20 Tokens or ATH).

After the last step of the buyer, the seller call SELLER_STEP_4_ACCEPT accepting the amount set by the buyer. The function will transfer the specified amount (Ether, ATH or ERC20 Tokens) to the seller address.

In conclusion, this contract doesn't provide any anonymity to the users.

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L402#L567

3.4. Token Contract Implementation

Severity: medium

Description

For better security, contracts implementing tokens should be completely autonomous to avoid external attacks. While the crowdsale phase is open an attacker can replace the crowdsale address in case if the owner private key gets hacked and use delivery to steal token from the contract. Private key hack is a common issue, but the severity is low only when the repercussion are only on the user itself.

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L204#L208

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L211#L220

3.5. Ethers Replenishment

Severity: medium

Description

Following the white paper "an investor who purchases 50,000 ATH or more, receives a lifetime share in the profit of the project, which is formed from 50% of all commission income of the project and is distributed automatically".

before that any user can supposedly transfer ether anonymously, he has to send ether to the contract first by calling replenishEth function. 2% of the sent ether is taking as fee and added to redemptionFund.

After the end of the ICO the investors will receive shares following their previous investment. The function automaticaly transfer ethers to all the concerned investors which will lead to a high gas consumption, making the contract not usable for users with a certain gas limit.

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L316#L346

recommendation

We strongly recommand to use a withdrawal pattern where the investors call a withdrawal function themselves to take their shares.

3.6. Misuse of Payable Specifier

Severity: medium

Description

BUYER_STEP_4 function should not be marked as payable since it doesn't manage user balance in case of nonzero msg.value, this can lead users to lose their assets.

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L505#L528

3.7. Seller Generated Passes

Severity: low

Description

SELLER_STEP_2_GET_PASS constant function only return the passes values of the last generated invoice, all previous invoices passes values cannot be return by that function, to get them the user has to read them from the raw blockchain data.

Code snippet

https://gist.github.com/yuriy77k/1f0588fa758a1b7bc8a3b4f38659997d#file-ethereum_anonymizer-sol-L426#L429

Conclusion

This project contains high severity operability issues that make its main purpose not applicable, users will not get any anonymity by transferring assets through this contract. Other issues with different severity levels have been highlighted.

This project doesn't fulfill its main goal, therefore not safe to be used.