Ethereum Naming Service Audited – ConsenSys Diligence – Medium

Audit Details

Audit Details — Issues found: 3 Critical, 1 Major, 5 Medium and 6 Minor.

The audit team found some interesting bugs during this audit. We expand on two of the finding here.

Front-running

Front-running is a pervasive issue on blockchain. It is really challenging to foresee all possible race conditions in the logic of the DApp. We previously published extensively on the fact that front-running is a very common type of security vulnerability in Ethereum DApps.

ENS has been one of the cutting edge DApps in the Ethereum ecosystem, starting back in April 2016 as EIP-137. ENS famously implemented Vickrey Auctions for initial registration of the domains through EIP-162, and used commit and reveal schemes to prevent domain sniping and front-running.

As mentioned before, making DApps resilient against front-running is hard. Even though the ETHRegistrarController smart contract uses a proper commit and reveal scheme to prevent front-running, failing to include even one crucial variable can lead to opening up the attack vector again. On the implementation under audit, the owner was not included in the committed message, making it vulnerable to front-running, which allowed the commitment transaction to be revealed and registered by any address.

The solution? include the right set of data the user is committing. You can read more about this specific issue and the remediation here: 
3.3 ETHRegistrarController.register is vulnerable to front running

Memory Corruption

This was an interesting find. On the last few days of the audit, when we thought we have gone through every piece of code in the scope, we decided to expand our search and look at the libraries used in the main contracts. buffer.sol was out of the scope but it was used throughout the main contracts.

buffer.init function was implemented as following:

function init(buffer memory buf, uint capacity) internal pure returns(buffer memory) {
if (capacity % 32 != 0) {
capacity += 32 - (capacity % 32);
}
// Allocate space for the buffer data
buf.capacity = capacity;
assembly {
let ptr := mload(0x40)
mstore(buf, ptr)
mstore(ptr, 0)
mstore(0x40, add(ptr, capacity))
}
return buf;
}

Note that memory is reserved only for capacity bytes, but the bytes actually requires capacity + 32 bytes to account for the prefixed array length. Other functions in Buffer assume correct allocation and therefore corrupt nearby memory.

You can read more about this issue and the remediation here: 3.1 Memory corruption in Buffer

LEAVE A REPLY

Please enter your comment!
Please enter your name here