EtherBots Security Audit Report

ETH_EtherBots_report.md

EtherBots Security Audit Report

1. Summary

EtherBots smart contract security audit report performed by Callisto Security Audit Department

2. In scope

Сommit hash 8f1f1752cb2942184df695e0442b04a38f0807ba.

• TestMintableNFT.sol.
• DetailedERC721.sol.
• ERC721.sol.
• Migrations.sol.
• MintableNonFungibleToken.sol.
• NonFungibleToken.sol.

3. Findings

In total, 3 issues were reported including:

• 2 low severity issues.

• 1 notes.

No critical security issues were found.

3.1. Wrong return value name

Severity: notes

Description

In functions name, symbol, totalSupply, balanceOf, ownerOf, tokenMetadata there are return value name is different from return value.

Recommendation

Change the return value name in brackets or delete it.

3.2. Token ID Re-approval

Severity: low

Description

Once a tokenID approved to an address the token owner cannot re-approve it to another address without resetting the approved address of the token to zero, this mechanism is used to partially solve ERC20 double withdrawal issue but in this case a token is non fungible once the token transferred from an address, the first owner do no posses it anymore and cannot reapprove it.

This implementation require two transactions to set the approved address to another address and can cause compatibility issue with Dapp that uses a non modified ERC721 standard approval function (check reference example for more information here).

Code snippet

https://github.com/RideSolo/NonFungibleToken/blob/master/contracts/NonFungibleToken.sol#L95

https://github.com/RideSolo/NonFungibleToken/blob/master/contracts/NonFungibleToken.sol#L102#L103

Recommendation

Remove the condition that block the owner from resetting approval address for a tokens id, since it does not add any security (just extra gas consumption and complication to reset the address).

3.3. Known vulnerabilities

Severity: low

Description

Same as ERC-20, ERC-721 is vulnerable to 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 the following code to the transfer(_to address, ...) function:

require( _to != address(this) );

4. Conclusion

The audited smart contract can be deployed. Only low severity issues were found during the audit.

5. Revealing audit reports

https://gist.github.com/yuriy77k/7a811c673d25995b8a57d14d56ab16be

https://gist.github.com/yuriy77k/c16cb068cb75489bb31303f3e9ff2ee9

https://gist.github.com/yuriy77k/d1a1f71ce701674f137d4946c415821a