Vulnerability Disclosure: LAND, October 2018
Full disclosure of a vulnerability found in the LAND contract
Last week, the security team at Decentraland responded to a vulnerability report from Agustín Aguilar regarding a bug in the LAND smart contract. The issue has been fixed and we can confirm that no LAND parcels were actually affected. This summary contains the technical details of the event.
What was the vulnerability?
The LANDRegistry is a proxy contract that takes the actual code from another contract. The ERC721 deployment that was compromised was deployed at the address 0x7623127801542906eaf18e4d32e51b5a331c281a
.
On October 24th, 2018, at approximately 20:00 GMT, an independent researcher reported a critical bug in how the _moveToken
method is implemented. The _moveToken
method is used internally for all transfers of tokens in the contract, and is able to call these three methods:
_removeAssetFrom (holder, assetId);
_clearApproval (holder, assetId);
_addAssetTo (to, assetId);
The vulnerability was generated because _removeAssetFrom
removes the value of _holderOf
from the assetId
, which causes ownerOf(assetId)
to become 0x0
.
Subsequently, _clearApproval
only comes into effect if ownerOf(assetId) == holder
, otherwise the call to _clearApproval
is ignored. This could have been detected previously replacing the if for a require, so the transfer would never work and the vulnerability would have been evident.
Resolution
The vulnerability was immediately repaired in the LAND smart contract through the following steps:
- The order of
_removeAssetFrom
and_clearApproval
in_moveToken
was reverted. - A privileged method to clear the operator of a parcel was added, in order to preemptively clear all current authorized operators.
- An integrity check was run to positively verify that the vulnerability was never exploited.
Thanks to Agustín Aguilar, from Ripio Credit Network, for the responsible disclosure of this vulnerability.