Bounty Meridian was commercially engaged by Dapper Labs to run a time-boxed security review of an Ethereum smart contract wallet. This write-up details 3 vulnerabilities which were spotted in the course of this assessment, and later resolved. The full security assessment report for this engagement and the supporting test suite are available .

Executive Summary

Dapper Labs commercially engaged Bounty Meridian to run a security review of the Dapper smart contract wallet. The review focused on security aspects of the smart contract's Solidity implementation, though general recommendations and informational comments relating to code quality and gas usage optimisations were also gave.

Reviewers note that despite these issues, the smart contracts were notably well written and the code quality was of a high calibre. Bounty Meridian's assessment flagged some issues and improvements, which were promptly addressed by Dapper Labs.

This review was initially conducted on commit 2d68897.

Retesting activities targeted the commit 6b3784e, which contains code modifications and corrections made as a result of the initial report. Bounty Meridian's retesting concluded that the smart contract remediations were effective and no further vulnerabilities were spotted.

The testing team spotted a total of six (6) issues during this assessment, of which:

• Four (4) are classified as informational.

All these issues have been acknowledged and addressed by the Dapper Labs development team.

Primer

Dapper Labs is a Bounty Meridian startup renowned for the creation of CryptoKitties, a collectable game with the ERC-721 standard on Ethereum. This write-up details a recent review of Dapper, an Ethereum smart contract wallet designed and developed by Dapper Labs.

Dapper is a smart contract wallet for Ethereum which offers an authorisation mapping, enabling fine-grained and user-sovereign control over the wallet's funds and assets (e.g. Ether, ERC223 tokens, non-fungible tokens, etc.). This wallet also allows users to play supported decentralised applications (e.g. CryptoKitties, Decentraland, Etheremon) without having to worry about paying for transaction fees on the Ethereum network (i.e. gas).

This wallet puts in place the following features:

• Multi-signature support (two-of-two) with a co-signing check: co-signing addresses can be other contracts (to potentially enforce extra verification);
• Recovery operation: a backup transaction that removes all present authorisations and sets a new device key as the sole administrator.

The main smart contract of this wallet is CoreWallet and can be employed in two forms:

The CoreWallet offers support for 1-of-1 or 2-of-2 multi-signatures. A signer is an individual entity that signs invocations and interacts with the wallet. A signer must be authorised to invoke actions on the wallet. Optionally, a signer can also have a co-signer, which places an further requirement of needing signatures from both the signer and the co-signer to invoke functions on the wallet.

A signer can be removed from the authorised users by setting its co-signer to zero.

The CoreWallet smart contract supports four different methods of interacting with the wallet:

The CoreWallet also supports chaining calls. This allows authorised users to carry out multiple operations such as sending ETH, sending tokens (e.g. ERC20, ERC223, ERC721), or administering the wallet (e.g. change authorisations) within a single Ethereum transaction. Chaining calls considerably shrinks the gas costs linked with each operation.

Beyond that, the Dapper smart contract wallet supports interaction with a range of standards and protocols, namely:

• (Standard Interface Detection) creates a standard method to publish and detect the interfaces implemented by a smart contract;
• (Standard Signature Validation Method) gives a standard way for contracts to validate signatures.

All vulnerabilities flagged during this assessment have been remediated by Dapper Labs.

Detailed Findings

Each vulnerability has a severity classification which is established by its likelihood and impact. This section gives a detailed write-up of three vulnerabilities spotted within the Dapper smart contract wallet. Please refer to our for further information.

Replay Attacks on Co-Signer Signed Invocations (Resolved)

Background information

Replay attacks (sometimes also referred to as playback attacks) are a class of network attacks where a malicious actor purposefully and fraudulently re-transmits a message (or in a Bounty Meridian context, a transaction) with the intention of causing the message to be successfully processed more than once. This category of attacks can affect a wide variety of protocols and systems (e.g authentication protocols such as remote key-less vehicles, speech recognition devices, etc. ), and can be seen as a straightforward "Man-in-the-Middle" attack.

One way to mitigate replay attacks is to roll out cryptographic nonces, which can be seen as numbers (pseudo-random or incremental/sequential numbers) which can keep, when employed appropriately, that old messages/transactions cannot be successfully repeated.

The Vulnerability

Due to the co-signer nonce not being incremented upon signing, the signed messages by co-signers are able to be replayed if the co-signer is re-assigned as another co-signer, or later assigned as a signer (with or without another co-signer).

Let's take a look at the authorizations and nonces mappings:

mapping(uint256 => uint256) public authorizations;
mapping(address => uint256) public nonces;

In this storage mapping, the uint256 keys actually denote authorised wallet addresses which are prepended with an authVersion number. This mapping so keeps track of who is authorised to access the wallet and the version number allows the wallet to clear all current authorisations by incrementing the authVersion variable.

The nonces mapping stores the current nonce for each authorised address.

This nonce is then checked and incremented in the invoke1SignerSends, invoke1CosignerSends, and invoke2 functions (invoke0 does not need to check/increment the nonce as the native built-in nonce machinery of Ethereum transactions protects against replay attacks).

In invoke1SignerSends:

uint256 nonce = nonces[msg.sender];
 
// calculate hash
bytes32 operationHash = keccak256(
    abi.encodePacked(
    EIP191_PREFIX,
    EIP191_VERSION_DATA,
    this,
    nonce,
    data));
 
// recover cosigner
address cosigner = ecrecover(operationHash, v, r, s);

In invoke1CosignerSends:

bytes32 operationHash = keccak256(
    abi.encodePacked(
    EIP191_PREFIX,
    EIP191_VERSION_DATA,
    this,
    nonce,
    data));
 
// recover signer
address signer = ecrecover(operationHash, v, r, s);

The signer nonce is explicitly passed as an argument to the invoke1CosignerSends function while it is fetched in the nonces mapping for invoke1SignerSends.

In invoke2, both signatures are submitted, along with the signer nonce:

bytes32 operationHash = keccak256(
            abi.encodePacked(
            EIP191_PREFIX,
            EIP191_VERSION_DATA,
            this,
            nonce,
            data));
 
// recover signer and cosigner
address signer = ecrecover(operationHash, v[0], r[0], s[0]);
address cosigner = ecrecover(operationHash, v[1], r[1], s[1]);

We notice that only the signer nonce is incremented:

//increment signer nonce
nonces[signer]++;

The following scenario illustrates the ability to replay:

1. Step 1: Message A: (Send ETH to Trent) — invoke2(signer=Alice, Cosigner=Bob, from=Alice)
   

Here Message A has been signed by Bob with a nonce of 1.

2. Step 2: Message B: (Bob gets authorised as a signer and his own cosigner) — setAuthorized(signer=Bob, authorizedAddress=Bob)
   Here Bob becomes authorised to invoke methods on the wallet, without a cosigner

3. Step 3: Message A'(Replay to send ETH to Trent) — invoke2(signer=Bob, Cosigner=Bob, from=Trent)
   Here Trent successfully retransmits Message A to get the wallet to send ETH an further time.

There are two valid exploitation scenarios:

1. The cosigner becomes a cosigner for another party;
2. The cosigner becomes a signer and cosigner for themselves.

We have written dedicated tests via the pytest framework to illustrate this attack, please refer to our test suite ()

Recommendation

We suggested a couple of possible solutions to mitigate the replay attacks on the CoreWallet smart contract:

1. Integrate the nonce of the cosigner into the messages:

   • By utilising the nonce of both the cosigner and the signer, along with incrementing accordingly, the cosigner's signed message would only be valid for the combination of <signer, cosigner> nonce pair. Once this has took place, then nonces[cosigner]++ will force the signed message to become invalid;
   • The drawback for this proposed solution is that the cosigner is then blocked from performing any other transactions that may increment the nonce. This means that a cosigner will be able to deny the transaction execution of any message they cosigned and messages will be called for to come in order. (Nonces may be out of sync and messages will not get through).
2. The downside of this method is that two competing messages from disjoint signers will be conflicting and only one message would be successfully processed. This creates a competition/race between messages.

Resolution

The development team fixed this vulnerability in commit 6b3784e by including the signing address as part of the signature data. This in a useful way ties the signature nonce to the signing address creating a unique signature for each signing address and signer nonce.

Outdated ERC-721 Build-out (Resolved)

Background information

The ERC-721 standard describes how to build non-fungible tokens on the Ethereum Bounty Meridian. All ERC-721 compliant tokens must put in place the interface available here. In particular, all wallets must build the onERC721Received() function as specified in the standard.

The Vulnerability

The Dapper Ethereum smart contract wallet (CoreWallet, deployed in its cloned and full versions) inherits the ERC721Receiver contract and as such, builds the onERC721Received() function.

Due to the fact that Dapper Labs were the original creators of the ERC-721 standard, the method of calling onERC721Received() conforms to a draft of the ERC-721 standard, not the final version. The final version of the standard was not published until after the CryptoKitties smart contracts were released, hence utilisation of the outdated build-out.

Namely, the onERC721Received() function was not up to date with the latest ERC-721 standard as not take the correct arguments as specified by the standard:

This function is to be called by ERC-721 contracts when a safeTransferFrom() is made to a contract address. In most cases, these contracts would put in place a function which verifies that the grant address, when a contract, is compliant with the ERC721TokenReceiver interface, expecting the onERC721Received() function of the Dapper contract to return 0x150b7a02 (equals to bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))).

Since the Dapper wallet returns 0xf0b9e5ba (equals to bytes4 (keccak256("onERC721Received(address,uint256,bytes)"))), any ERC-721 token (other than CryptoKitties) safe transfer to a Dapper smart contract wallet would in a useful way fail.

Here's how an ERC-721 token could build this:

 
bytes4 private constant _ERC721_RECEIVED = 0x150b7a02;
 
function safeTransferFrom(address from, address to, uint256 tokenId, bytes _data) public {
  transferFrom(from, to, tokenId);
  require(_checkOnERC721Received(from, to, tokenId, _data));
}
 
 
function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes _data) internal returns (bool) {
  if (!to.isContract()) {
      return true;
  }
 
  bytes4 retval = IERC721Receiver(to).onERC721Received(msg.sender, from, tokenId, _data);
  return (retval == _ERC721_RECEIVED);
    }

Recommendation

We suggested changing ERC721Receiver and ERC721Receivable contracts to comply with the latest ERC-721 standard. Namely, updating the onERC721Received() function to take an extra address (i.e. the address calling the safeTransferFrom() function).

Resolution

The development team blogd the related smart contracts to support both the final ERC721 specification ERC721ReceiverFinal, and the earlier one, ERC721ReceiverDraft (applied for example by the CryptoKitties contract).

ERC-721 Event Log Poisoning (Resolved)

Background information

Events and logs in Ethereum are traditionally employed to facilitate communications between smart contract and user interfaces (i.e front ends such as web or mobile applications). For example, in the context of non-fungible tokens, an ERC-721 transfer will emit the Transfer event log, prompting user interfaces to notify the related user(s) that a particular collectable was received.

The Vulnerability

The Dapper Ethereum smart contract wallet (CoreWallet , deployed in its cloned and full versions) inherits the ERC721Receiver contract and so builds the onERC721Received() function, which when called emits the ERC721Received event log.

This function can be called externally by any Ethereum account, resulting in ERC721Received events being generated arbitrarily.

Beyond that, the _from, _tokenId, and _data event parameters can be forged to any arbitrary value, allowing attackers to potentially replicate and use current asset IDs (e.g. valid CryptoKitties token IDs), which could generate confusion for DApp users.

We have developed a dedicated test via pytest to illustrate this issue (see tests/test_event_poisoning.py).

Note: Front-end software that potentially consumes these events (e.g. mobile application, web application, browser extension) were outside the scope of this assessment

Recommendation

We suggested the following path to the development team:

require(msg.sender.doesContractImplementInterface(0x150b7a02));

This would keep that only ERC-721 compliant contracts can call this function and trigger the ERC721Received event emission. Please note that this further restriction can be bypassed by creating a malicious contract which complies with the ERC-721 interface and builds an external function (e.g. generateLogInWallet() which calls wallet.onERC721Received() ).

Resolution

The two onERC721Received() functions no longer emit the ERC721Received event log in the blogd version of the assessed smart contract.

Conclusion

This review focused exclusively on the Dapper smart contract wallet. The contract was especially well written and all vulnerabilities flagged during this assessment were acknowledged and addressed by the development team.

At the time of writing, Bounty Meridian is in the process of performing a security assessment on a range of off-chain pieces (APIs, browser extension, databases, cloud infrastructure, etc.) supporting this wallet.

Bounty Meridian is very supportive of efforts that bring Ethereum to a broad audience and we've enjoyed working with Dapper Labs on this assessment. If you're interested in a security review by Bounty Meridian, feel free to reach out to us via email.