The Sandbox — ASSET Smart-Contract Security Update
UPDATE January 16th, 2021: Migration is complete! All the ASSETS have been migrated, and trading has been re-enabled on marketplaces.
The new verified ASSET smart-contract address is 0xa342f5D851E866E18ff98F351f2c6637f4478dB5
To our community, thank you for being a part of The Sandbox community. Please note that no funds are lost and there is no action required on your end.
For questions, comments and concerns, please join our Discord or Telegram
TL;DR
- The Sandbox ASSET smart-contract was vulnerable to a duplication issue;
- All others smart-contracts are NOT affected; neither SAND nor LAND smart-contracts are at risk;
- This issue has not been exploited by any malicious user;
- All major marketplaces trading The Sandbox ASSETS were notified on January 13th, 2021 and already halted transfers of ASSETS on January 16th, 2021 at 00.01 UTC;
- We have redistributed ASSETS to 2,684 unique users who hold 115,926 copies of 87 unique ASSETS (no user costs were or will be incurred)
- Distribution has been completed on January 16th at 6:00PM UTC
Finally, we thank all our partners and the community for their trust and support of The Sandbox’s ongoing development.
General Overview
The Sandbox smart-contracts were extensively audited by two top security firms: Certik and Solidified and received high security scores. These audits can be accessed publicly on our Github repository. The ASSET smart-contract was originally audited by Solidified on September 4th, 2019 and involved three auditors on a 2-week long audit engagement.
Therefore, we decided to take immediate action and fix the issue, as the following timeline demonstrates.
Timeline
- On Jan 4th, Maurelian, a security researcher at Optimism (Medium https://medium.com/@maurelian / Twitter @maurelian_) notified us of the vulnerability
- On Jan 5th, we carried out internal tests and confirmed 100% the reproducibility of the issue
- Between Jan 5th and 14th, we worked on a fix and submitted it for audit again by Solidified security auditors to confirm the fix didn’t introduce new vulnerabilities
- On Jan 13th, we notified marketplaces (OpenSea, Arkane Marketplace, Lootex, Playdapp, iNFT) to halt trading of Assets
- On Jan 15th, we deployed the new fixed smart contract
- On Jan 16th, at 12:00PM UTC, we took a snapshot of the blockchain
- We are now redistributing the ASSET copies to all holders
What happened
The bug is similar to https://bzx.network/blog/incident and was discovered present in several other projects, but live on only two.
- It allows someone to send tokens to themselves to artificially increase their token balance by that same amount.
- This only happens when users use the batchTransfer function which, for optimization purposes, accumulated the result of a transfer in temporary memory variables.
- When it was time to write to storage, the following happened :
_packedTokenBalance[from][bin] = balFrom;
_packedTokenBalance[to][bin] = balTo;
See: https://github.com/thesandboxgame/sandbox-smart-contracts/blob/88615d4c9cdde260b7157537eedadb4c7e0aa8f5/old_src/Asset/ERC1155ERC721.sol#L452-L453 and https://github.com/thesandboxgame/sandbox-smart-contracts/blob/88615d4c9cdde260b7157537eedadb4c7e0aa8f5/old_src/Asset/ERC1155ERC721.sol#L471-L472
- balFrom is the new in-memory accumulated balance for the sender, which is basically the initial balance minus the amount transferred. balTo is the new in-memory accumulated balance for the receiver, which is basically the initial balance plus the amount transferred
- When from == to, the first line will store balFrom to storage reducing the balance of the user. But then the second line overwrites it with the balTo which basically increases the user’s balance by that amount instead.
- If the order of the lines were reversed, the reverse would have been true and users would basically be at a loss when sending tokens to themselves.
What we did to fix it :
The simplest and safest fix we could come up with was to simply skip storage manipulation when `from == to`. But we also had to ensure the TransferBatch could not simply emit the transfer of balances that the owner did not own.
The fix has been reviewed by Solidified and the audit can be found on our Github repository — https://github.com/thesandboxgame/assetbug/raw/asset_bug/documentation/audits/Audit%20Report%20-%20Sandbox%20Asset%20Bug%20Fix%20%5B15.01.2020%5D.pdf
We also took the opportunity to make the ASSET contract upgradeable using Open Zeppelin contracts.
This decision was not taken lightly; however since we already had admin functions that would enable The Sandbox's top admin (multisig owner) to remove ownership of all users' tokens, we decided to make the new contract upgradeable:
- To enable swifter recovery when a vulnerability is discovered (though we do not believe there are any remaining in the Asset contract)
- To enable future improvement, such as the ability to perform code changes and avoid having to redo once again a redeployment
We had to gather user data for affected ASSET owners, together with their balances, and redeploy the new ASSET contract with that information. We are now migrating all the data and, as mentioned above, we have completed the restoration of all user ASSETS by January 16th at 6:00PM UTC.
The new verified ASSET smart-contract address is 0xa342f5D851E866E18ff98F351f2c6637f4478dB5
Learnings / What will we do in the future?
Since the ASSET smart-contract was originally deployed we have significantly improved our tooling. We now use HardHat (http://hardhat.org) and its plugin ecosystem, which is helping us to improve our testing and ensure good coverage. We set up a test coverage report recently, which will be fully enabled soon.
It is worth noting that the bug could have been caught by testing and while we had a test for that edge it did not perform the necessary check. Indeed, test coverage tools do not ensure the tests are actually catching all vulnerabilities. While it is far better to have coverage statistics than nothing, there are other tools that will allow us to check if our tests are actually optimized for catching similar issues in future. One tool in particular : https://github.com/JoranHonig/vertigo would help to achieve this by mutating contract code and checking to see if tests are actually failing when such mutation occurs. We would like to help this project become compatible with our tooling.
Closing words
Once again, thank you for reading and being part of The Sandbox community.
We look forward to your continued support and appreciate your patience as we deal with this delicate manner.
Thank you, Stay safe!
The Sandbox Team
