Introduction

CoinFabrik was asked to audit the contracts for the DoubleDice project. First we will provide a summary of our discoveries and then we will show the details of our findings.

Scope

The contracts audited are from the https://github.com/DoubleDice-com/doubledice-platform git repository. The first iteration of this audit is based on the commit 5bd440da26ebb258698d684ac2281f38aa607c32. 

Fixes were checked on commits de79f3d4741f34a0a1b45ae049e27a33d9a88516, 6def4aa8a3a8d582bd9943e772bfe3f0a2c2671f,  b5a38f2d8d6f9958af906c69d627a8b5f9ab4439,
e0fca9c9f1eaa090b0c9398ea0fdc9f7da9114ea, 09c5c068042125b8c42f8be9f1d9a2b6b5efada6, and 41327e967c5a644e405480b4c3750f8919bb0026.

The second iteration of this audit is based on the commit 3fea8d3a280a06b14c777b180ca9a3b8f4950587. Fixes were checked in commit b2c94d7798668cb368993dac3fd0ca74a21002fe.

The audited contracts are:

• contracts/BaseDoubleDice.sol: core VF lifecycle
• contracts/ForkedERC1155UpgradeableV4_5_2.sol: Multi-token implementation fork
• contracts/ChallengeableCreatorOracle.sol: Challengeable VF resolver
• contracts/SimpleOracle.sol: Simple VF resolver
• contracts/CreationQuotas.sol: VF Quota limiter
• contracts/DoubleDice.sol: Top contract
• contracts/VirtualFloorMetadataValidator.sol: VF parameters validator
• contracts/MultipleInheritanceOptimization.sol: Optimization
• library/FixedPointTypes.sol: Fixed types implementation
• library/VirtualFloors.sol: VF main contract

The scope of the audit is limited to those files. No other files in this repository were audited. Its dependencies are assumed to work according to their documentation. Also, no tests were reviewed for this audit.

Analyses

Without being limited to them, the audit process included the following analyses:

• Arithmetic errors
• Outdated version of Solidity compiler
• Race conditions
• Reentrancy attacks
• Misuse of block timestamps
• Denial of service attacks
• Excessive gas usage
• Missing or misused function qualifiers
• Needlessly complex code and contract interactions
• Poor or nonexistent error handling
• Insufficient validation of the input parameters
• Incorrect handling of cryptographic signatures
• Centralization and upgradeability

Summary of Findings

We found 1 critical issue, 2 medium issues and a minor issue which were fixed by the development team. Also, several enhancements were proposed.

Security Issues

Privileged Roles

These are the privileged roles that we identified on each of the audited contracts.

OPERATOR_ROLE

This role cuts across various contracts (BaseDoubleDice, ChallengeableCreatorOracle and CreationQuotas)

An address with the operator role can:

• Change the internal state of a VF from Active to Claimable_Refunds_Flagged.
• End challenge when it is spawned with a final outcome.
• Adjust Creation Quotas.
• Set the result if the VF creator does not do so.

BaseDoubleDice

DEFAULT_ADMIN_ROLE

This role is allowed to pause/unpause the contract, configure platform parameters.

ChallengeableCreatorOracle

VF_CREATOR

This role is not defined as such, but the only address that can execute the function setResult(), line 132, is the creator of the VF.

Security Issues Found

Severity Classification

Security risks are classified as follows:

• Critical: These are issues that we manage to exploit. They compromise the system seriously. They must be fixed immediately.
• Medium: These are potentially exploitable issues. Even though we did not manage to exploit them or their impact is not clear, they might represent a security risk in the near future. We suggest fixing them as soon as possible.
• Minor: These issues represent problems that are relatively small or difficult to take advantage of but can be exploited in combination with other issues. These kinds of issues do not block deployments in production environments. They should be taken into account and be fixed when possible.

Issues Status

An issue detected by this audit can have four distinct statuses:

• Unresolved: The issue has not been resolved.
• Acknowledged: The issue remains in the code but is a result of an intentional decision.
• Resolved: Adjusted program implementation to eliminate the risk.
• Partially resolved: Adjusted program implementation to eliminate part of the risk. The other part remains in the code but is a result of an intentional decision.
• Mitigated: Implemented actions to minimize the impact or likelihood of the risk

Critical Severity Issues

CR-01 Block.timestamp Manipulation

Location: 

• contracts/BaseDoubleDice.sol:228,318
• contracts/ChallengeableCreatorOracle.sol:107-156
• library/VirtualFloors.sol:23,31,83

Malicious miners can manipulate the block’s timestamp value to gain advantages, so a developer can’t rely on the preciseness of the provided timestamp. This is particularly important in short-duration betting events, where the variation of this value may be enough to manipulate them. For more information see SWC-116 (https://swcregistry.io/docs/SWC-116).

Recommendation

A way to mitigate the lack of precision of this value in betting events is to make sure the event is closed some minutes before the actual event, so the bets cannot be manipulated.

Status

Fixed on commits: b5a38f2d8d6f9958af906c69d627a8b5f9ab4439, 09c5c068042125b8c42f8be9f1d9a2b6b5efada6, and 41327e967c5a644e405480b4c3750f8919bb0026.

Medium Severity Issues

ME-01 Insecure Token Transfer

Location: 

• contracts/BaseDoubleDice.sol:527,562

The ERC20 transfer() function does not always revert in the case of error, sometimes it just returns false. In this particular contract the code does not check for the return value, thus ignoring if the transfer was successful or not.

Recommendation

Replace the offending function with the safer safeTransfer() function that automatically asserts in the case of error.

Status

Fixed on commit: e0fca9c9f1eaa090b0c9398ea0fdc9f7da9114ea

ME-02 Reentrancy Point in BaseDoubleDice.sol

Location: 

• contracts/BaseDoubleDice.sol:310

The function commitToVirtualFloor() in the BaseDoubleDice.sol contract is susceptible to reentrancy when calling token.safeTransferFrom(). Notice that if the attacker controls the token contract, he can implement reentrancy attacks in the safeTransferFrom() call at BaseDoubleDice.sol:310 function by calling commitToVirtualFloor() recursively.  The attacker can use the reentrancy to skip  the update of the outcomeTotals variable at baseDoubleDice.sol:331.

Recommendation

Use a reentrancy guard or place reentrancy checks. 

Status

Reentrancy guards added, additionally all other reentrancy risks were addressed on commit 6def4aa8a3a8d582bd9943e772bfe3f0a2c2671f.

Minor Severity Issues

MI-01 Floating Pragma

Location: 

• contracts/ForkedERC1155UpgradeableV4_5_2.sol:4

Contracts should be deployed with the same compiler version that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Recommendation

Lock the pragma version, replacing pragma solidity ^0.8.0; with a specific patch, preferring the most updated version. For example, pragma solidity 0.8.12;.

Status

Fixed on commit de79f3d4741f34a0a1b45ae049e27a33d9a88516.

Enhancements

These items do not represent a security risk. They are best practices that we suggest implementing.

Table

Details

EN-01 Revert Transactions As Soon As Possible

Location: 

• doubledice-platform/contracts/BaseDoubleDice.sol:492

In the _resolve() function, some actions are first performed and then it is queried if the contract is paused.

Recommendation
It would be better to first check if the contract is paused, so that the gas expense is lower.

Status

Implemented. (commit: fb09655d9b0a994cf12ec71476cb01117a0ecc55).

EN-02 Not Default Action With Invalid States

Location: 

• doubledice-platform/contracts/library/VirtualFloors.sol:49

The final else of the state() function returns the value Claimable_Refunds_Flagged. Currently it is not a bug, since it uses all the values ??of the VirtualFloorInternalState enum, but if an extra value is added in the future, it could lead the contract to return incorrect states. 

Recommendation

Use else if with the condition and then put a revert or assert on the else.

Status

Implemented. In the else statement it is validated with an assert.
(commit: fb09655d9b0a994cf12ec71476cb01117a0ecc55).

Other Considerations

The considerations stated in this section are not right or wrong. We do not suggest any action to fix them. But we consider that they may be of interest for other stakeholders of the project, including users of the audited contracts, owners or project investors.

Centralization

In the Privileged Roles section, the DEFAULT_ADMIN_ROLE, OPERATOR_ROLE and VF_CREATOR system actors were described.
DEFAULT_ADMIN_ROLE has the capability to configure/change various parameters of the platform such as fees and beneficiary address. It can also pause/unpause the contract. Particularly within these functionalities that can be paused, is the claim for payments and refunds.
Each VF_CREATOR centralizes setting its own result.
Finally, OPERATOR_ROLE can change the internal state of a VF from Active to Claimable_Refunds_Flagged, end challenge when it is spawned with a final outcome and set the result if the VF creator does not do so. 

Changelog

• 2022-03-15 – Initial report based on commit 5bd440da26ebb258698d684ac2281f38aa607c32.
• 2022-03-22 – Fixes checked on commits de79f3d4741f34a0a1b45ae049e27a33d9a88516, 6def4aa8a3a8d582bd9943e772bfe3f0a2c2671f,  b5a38f2d8d6f9958af906c69d627a8b5f9ab4439,
  e0fca9c9f1eaa090b0c9398ea0fdc9f7da9114ea, 09c5c068042125b8c42f8be9f1d9a2b6b5efada6, and 41327e967c5a644e405480b4c3750f8919bb0026.
• 2022-03-28 – Second iteration of this audit, based on commit 3fea8d3a280a06b14c777b180ca9a3b8f4950587.
• 2022-03-31 – Fixes checked on commit b2c94d7798668cb368993dac3fd0ca74a21002fe.

Disclaimer: This audit report is not a security warranty, investment advice, or an approval of the DoubleDice project since CoinFabrik has not reviewed its platform. Moreover, it does not provide a smart contract code faultlessness guarantee.