Fluidity Audit Report

• Code Assessment
  • We evaluate the overall quality of the code and comments as well as the architecture of the repository.
  • We help the project dev team improve the overall quality of the repository by providing suggestions on refactorization to follow the best practice of Web3 software engineering.

• Code Logic Analysis
  • We dive into the data structures and algorithms in the repository and provide suggestions to improve the data structures and algorithms for the lower time and space complexities.
  • We analyze the hierarchy among multiple modules and the relations among the source code files in the repository and provide suggestions to improve the code architecture with better readability, reusability, and extensibility.

• Business Logic Analysis
  • We study the technical whitepaper and other documents of the project and compare its specification with the functionality implemented in the code for any potential mismatch between them.
  • We analyze the risks and potential vulnerabilities in the business logic and make suggestions to improve the robustness of the project.

• Access Control Analysis
  • We perform a comprehensive assessment of the special roles of the project, including their authorities and privileges.
  • We provide suggestions regarding the best practice of privilege role management according to the standard operating procedures (SOP).

• Off-Chain Components Analysis
  • We analyze the off-chain modules that are interacting with the on-chain functionalities and provide suggestions according to the SOP.
  • We conduct a comprehensive investigation for potential risks and hacks that may happen on the off-chain components and provide suggestions for patches.

1. Potential signature replay attack and vulnerability.

   Severity	High
   Source	contracts/ethereum/contracts/Token.sol#L257;
   Commit	3578295;
   Status	Resolved in commit b325a91.

   • Description

     The signature lacks fields for validation (e.g., nonce, timestamp, and token address). This leads to potential signature replay attacks.

     When constructing signatures, only fields winner, winAmount, firstBlock, lastBlock are used. Fluid token supports multiple principle tokens and multiple platforms. It does not specify which fluid token contract it can only be used on. Thus, one signature can be replayed on different fluid token contracts.

   • Exploit Scenario

     1. Mallory uses the signature that is designed to claim manualReward() of USDC to claim manualReward() for USDT;

     2. Mallory uses the signature that is designed to claim manualReward() of USDC to claim manualReward() for a second time.
   • Recommendations

     We suggest constructing signatures according to EIP-712. Fields like chain-id and contract address should be included in messages to prevent signature replay on different fluid tokens. Besides, it can also increase signature readability since most wallets support decoding messages that follow the EIP-712 standard.

   • Results

     Resolved in commit b325a91. The fluidity team added contractAddress and chainId to construct the signatures.

2. Updating the wrong variable.

   Severity	High
   File	contracts/ethereum/contracts/Token.sol#L292;
   Commit	26e9450;
   Status	Resolved in commit 5d88bec.

   • Description

     The value amount / userAmountMinted + amount is assigned to the wrong variable userLastMintedBlock_. Replace userLastMintedBlock_[msg.sender] with userMint.

     if (userHasMinted) {
       userMint = amount;
     } else {
       userMint = userAmountMinted_[msg.sender] + amount;
     }
     require(userMint <= userMintLimit_, "mint amount exceeds user limit!");

   • Exploit Scenario

     N/A

   • Recommendations

     Change userLastMintedBlock_[msg.sender] to userMint.

   • Results

     Resolved in commit 5d88bec.

2. Lack of function to disable emergency mode

   Severity	High
   File	contracts/ethereum/contracts/WorkerConfig.sol; contracts/ethereum/contracts/Token.sol;
   Commit	26e9450;
   Status	Resolved in commit 6d8b5ab and e181cef;

   • Description

     The entire protocol will be suspended when it is in emergency mode. However, there is no function to disable the emergency mode and resume working again.

   • Exploit Scenario

     N/A

   • Recommendations

     Add a function to disable the emergency mode.

   • Results

     Resolved in commit 6d8b5ab and e181cef.

1. Lack of two-step process for critical operations

   Severity	Medium
   Source	contracts/ethereum/contracts/Token.sol#L111;
   Commit	3578295;
   Status	Resolved in commit f56127a.

   • Description

     The critical state variable rngOracle_ can be changed by calling function Token.updateOracle(), which requires the caller to be rngOracle_ itself and immediately sets the rngOracle_ to a new address. Value change of the critical state variable in a single step is error-prone and can lead to irrevocable mistakes.

     rngOracle_ is a critical state variable related to critical operations Token.batchReward() and Token.manualReward(), which sets the winners and reward amounts.

     As the oracle rngOracle_, it can set oracle to a new address by calling function Token.updateOracle(). The function requires the function call to be made from rngOracle_ and set the oracle to a new address directly. Once the oracle is set to a new address, only the new oracle address has the authority to update the oracle address. This process is single-step and irrevocable. The new oracle has the right to set the winners and rewards amounts. And if the oracle is set to a zero address, the normal operations of the protocol are halted directly.

   • Exploit Scenario

     As the oracle contract, it accidentally set the oracle address of contract Token.sol to a random address. The owner of the new oracle address can call function Token.batchReward() and Token.manualReward() to set any award amounts to any users.

   • Recommendations

     Implement a two-step process for updating the oracle address. The oracle proposes a new address and the oracle is only updated after the proposed new oracle address executes a call to confirm the change.

   • Results

     Resolved in commit f56127a. A set and claim two-step process was implemented to update the oracle address

2. Lack of input data validation

   Severity	Medium
   Source	contracts/ethereum/contracts/Token.sol#L111;
   Commit	3578295;
   Status	Acknowledged.

   • Description

     Function Token.updateOracle() lacks of zero-value check on newOracle. The protocol's normal operations can be halted if the oracle is set to zero address.

     Function Token.updateOracle() sets critical state variable in single step and lack of zero-value check on parameter newOracle. Once the oracle is set to the zero address. The contracts can no longer set winners and reward amounts since these states can only be updated by oracle. In this situation, it directly halts the normal operation of the protocol.

   • Exploit Scenario

     When the oracle account sets the oracle to the zero address, winners and reward amounts can no longer be set.

   • Recommendations

     Add input validations such as the zero-value check.

   • Results

     Acknowledged.

3. Lack of checking in signature recovery

   Severity	Medium
   Source	contracts/ethereum/contracts/Token.sol#L257;
   Commit	3578295;
   Status	Resolved in commit cac3886;

   • Description

     Lack of check on s range before feeding it to ecrecover(hash, v, r, s) function. This results in potential failure in claim reward via manualReward due to signature recovery failure.

     According to EIP-2, all transaction signatures whose s-value is greater than secp256k1n/2 are now considered invalid. A check to make sure s value is within range (0, secp256k1n ÷ 2 + 1) needs to be added to prevent signature recovery failure.

   • Exploit Scenario

     The input s is greater than secp256k1n/2 results in the failure of signature recovery and calls to manualReward fails.

   • Recommendations

     We recommend using OpenZepplin’s cryptography library.

   • Results

     Resolved in commit cac3886. fluidity team modified it based on openzeppelin's ECDSA library.

4. Insufficient check for the block range when distributing rewards

   Severity	Medium
   Source	contracts/ethereum/contracts/Token.sol;
   Commit	3578295;
   Status	Acknowledged.

   • Description

     Insufficient check for firstBlock and lastBlock in batchReward() and manualReward() could result in the rewards being distributed more than one time in a block range.

     For function batchReward(), there is no check against the block range of rewarded blocks. Rewards can be distributed to the same block range with the same winners again if workers in the server with oracle signing rights distribute rewards incorrectly.

     For function manualReward(), there is only one check for firstBlock and no check for lastBlock. winAmount can be added to manualRewards_[winner] and rewards will be distributed to users more than one time. This can result in manualRewards_[winner] being bigger than it should be and users get more rewards than they should.

     Checks should be enforced on the smart contract side as workers in the server can work incorrectly or be exploited.

   • Exploit Scenario

     lastRewardedBlock_ is 9.

     manualReward is called with firstBlock 10 and lastBlock 20 for winner Alice. Rewards are distributed to Alice.

     Then manualReward was called with firstBlock 10 and lastBlock 20 for winner Alice. Rewards are distributed to Alice again.

     Alice gets more rewards than she should.

   • Recommendations

     Enforce checks for block range on the smart contract side.

   • Results

     Acknowledged.

     Reply from Fluidity team:

     “We think manualReward is safe given the spooler implementation.”

5. Lack of important variables update function

   Severity	Medium
   File	contracts/ethereum/contracts/WorkerConfig.sol contracts/ethereum/contracts/Token.sol
   Commit	26e9450;
   Status	Acknowledged;

   • Description

     operator_ and emergencyCouncil_ are two main privileged roles, these two variables are set when initializing the smart contract. But these two variables can not be changed after initializing. In some cases, the smart contract owner may want to update it.

   • Exploit Scenario

     N/A

   • Recommendations

     Add a two-step process to change operator_ and emergencyCouncil_ .

   • Results

     Acknowledged.

     Reply from Fluidity team:

     “We don’t intend to change the operator or emergency council in production as they are both multisigs with stringent requirements.”

2. Lack of a two-step process for critical operations.

   Severity	Medium
   File	contracts/ethereum/contracts/Token.sol;
   Commit	26e9450;
   Status	Acknowledged;

   • Description

     The critical state variable operator_ can be changed by calling function Token.updateOperator(), which requires the caller to be operator_ itself and immediately sets the operator_ to a new address. Value change of the critical state variable in a single step is error-prone and can lead to irrevocable mistakes.

     operator_ is a critical state variable related to critical operations Token.enableEmergencyMode() suspends the protocol, and Token.batchReward() and Token.manualReward(), which sets the winners and reward amounts.

     As the operator operator_, he/she can set the operator to a new address by calling function Token.updateOperator(). The function requires the function call to be made from operator_ and set the operator to a new address directly. Once the operator is set to a new address, only the new operator address has the authority to update the operator address. This process is single-step and irrevocable. The new operator has the right to set the winners and rewards amounts. And if the operator is set to a zero address, the normal operations of the protocol are halted directly.

   • Exploit Scenario

     As the operator contract, it accidentally set the operator address of contract Token.sol to a random address. The owner of the new operator address can call function Token.batchReward() and Token.manualReward() to set any award amounts to any users.

   • Recommendations

     Implement a two-step process for updating the operator address. The operator proposes a new address and the operator is only updated after the proposed new operator address executes a call to confirm the change.

   • Results

     Acknowledged.

     Reply from Fluidity team:

     “We felt that the nature of the operator/emergency council (being multisigs) would prevent manual error from cropping up in practice along these lines. We will implement a process with the members of the multisig to manually verify the call by shifting the generated input to the safe to an ABI-decorated call and add some extra logging.”

3. Lack of emergency mode check for critical operations.

   Severity	Medium
   File	contracts/ethereum/contracts/Token.sol;
   Commit	26e9450;
   Status	Acknowledged.

   • Description

     The protocol is supposed to be paused after activating the emergency mode. However, critical operation rc20Out() is not protected by the emergency mode. Attackers are still able to execute the functions and get tokens from the pool by executing the function erc20Out() in the emergency mode of the protocol.

   • Exploit Scenario

     N/A.

   • Recommendations

     Add emergency control for function and erc20Out().

   • Results

     Acknowledged.

     Reply from Fluidity team:

     We wanted our users to be able to withdraw their funds but not deposit them in an emergency situation, so we didn’t disable erc20Out.

1. Lack of event emission for critical operations

   Severity	Low
   Source	contracts/ethereum/contracts/Token.sol#L111;
   Commit	3578295;
   Status	Resolved in commit e8d0a51.

   • Description

     critical function updateOracle lacks event emission. Event emissions are important for monitoring contracts and detecting suspicious behaviour.

     There is no event emission upon the value change of critical state variable rngOracle_ in function updateOracle().

     Operations that transfer value or perform critical operations should trigger events so that users and off-chain monitoring tools can account for important state changes. Events are important for monitoring contracts and detecting suspicious behavior.

   • Exploit Scenario

     N/A

   • Recommendations

     Emit event when updating critical state variable rngOracle_ in function updateOracle().

     event UpdateOracle(address indexed oldOracle, address indexed newOracle);
     /// @notice updates the trusted oracle to a new address
         function updateOracle(address newOracle) public {
             require(msg.sender == rngOracle_, "only the oracle account can use this");
     				emit UpdateOracle(rngOracle_, newOracle);
             rngOracle_ = newOracle;
         }

   • Results

     Resolved in commit e8d0a51.

2. Lack of event emission for critical operations.

   Severity	Low
   File	contracts/ethereum/contracts/Token.sol;
   Commit	26e9450;
   Status	Resolved in commit 6d8b5ab;

   • Description

     Critical functions setRestrictions ,updateWorkerConfig, updateRewardQuarantineThreshold,updateMintLimits, and enableMintLimits lack event emission. Event emissions are important for monitoring contracts and detecting suspicious behavior. Operations that transfer value or perform critical operations should trigger events so that users and off-chain monitoring tools can account for important state changes. Events are important for monitoring contracts and detecting suspicious behavior.

   • Exploit Scenario

     N/A.

   • Recommendations

     Add events for these critical functions.

   • Results

     Resolved in commit 6d8b5ab. Events are added for critical operations.

1. Floating solidity pragma version

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol;
   Commit	3578295;
   Status	Resolved in commit fa89c3e.

   • Description

     Current smart contracts use ^0.8.11. And compilers within versions≥ 0.8.11 and <0.9.0 can be used to compile those contracts. Therefore, the contract may be deployed with a newer or latest compiler version which generally has higher risks of undiscovered bugs.

     It is a good practice to fix the solidity pragma version if the contract is not designed as a package or library that will be used by other projects or developers.

     Reference: SWC-103.

   • Exploit Scenario

     N/A

   • Recommendations

     Use the fixed solidity pragma version.

   • Results

     Resolved in commit fa89c3e.

2. State variable visibility unspecified

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol;
   Commit	3578295;
   Status	Resolved in commit fa89c3e.

   • Description

     Some state variables’ visibilities are not specified. Labelling the visibility explicitly makes it easier to catch incorrect assumptions about who can access the variable.

     Visibility unspecified state variables:

     • initialized_

     • pool_

     • rngOracle_

     • rewardedBlocks_

     • lastRewardedBlock_

     The above state variables’ visibilities will be the default visibility internal. According to the naming style of Token.sol, private or non-public variable is named as variableName_, a name with an underscore suffix.

     All private state variables have public getter functions and some of those Visibility-unspecified variables do not have public getter functions. It would not be easy to obtain those variables’ values if they were meant to be publicly accessible.

     Specifying variables’ visibility can improve code readability and makes it easier to catch incorrect assumptions about the visibility of those variable.

   • Exploit Scenario

     N/A

   • Recommendations

     Explicitly specify all the state variable’s visibility.

   • Results

     Resolved in commit fa89c3e.

3. Inconsistent naming style

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol#L183;
   Commit	3578295;
   Status	Acknowledged.

   • Description

     Function naming style is inconsistent in contract Token.sol. A consistent naming style can improve code readability and make it easier to catch incorrect assumptions about the visibility of the function.

     In contract Token.sol, most of the functions with non-public visibilities are named as _functionName, function name with an underscore prefix. However, the internal function rewardFromPool and recover do not follow this naming style.

     Inconsistent naming style damages the code readability and can easily make people have false visibility assumptions of the function.

   • Exploit Scenario

     N/A

   • Recommendations

     Change function name from rewardFromPool to _rewardFromPool.

   • Results

     Acknowledged.

4. Redundant assertions for arithmetic operations in Solidity ≥ 0.8

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol#L135;
   Commit	3578295;
   Status	Resolved in commit b4eb458.

   • Description

     The overflow check require statement is unnecessary. Arithmetic operations revert on underflow and overflow since pragma version 0.8.0.

     Inside the function Token.erc20In(), it has a require statement that checks if the arithmetic operation will overflow.

     However, this require statement is unnecessary. Arithmetic operations revert on underflow and overflow since pragma version 0.8.0. No need for an extra arithmetic overflow check, which also costs extra gas consumption.

   • Exploit Scenario

     N/A

   • Recommendations

     Remove the unnecessary arithmetic overflow check for better gas optimization.

   • Results

     Resolved in commit b4eb458.

5. Shadowed variable declarations

   Severity	Informational
   Source	microservice-ethereum-worker-server/main.go#L145-L145;
   Commit	3578295;
   Status	Acknowledged.

   • Description

     The two environment variables for ethFluxAddress_ and tokenFluxAddress_ are the same. Need to remind Fluidity to double-check.

     ethFluxAddress_        = os.Getenv(EnvAuroraTokenFluxAddress)
     tokenFluxAddress_      = os.Getenv(EnvAuroraTokenFluxAddress)

   • Exploit Scenario

     N/A

   • Recommendations

     1. Double-check to confirm.

     2. Add a comment in the code saying that the two environment variables are purposely the same.
   • Results

     Acknowledged.

6. Magic numbers

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol#L200;
   Commit	3578295;
   Status	Resolved in commit 2600ff8.

   • Description

     Magic number 1 is used to flag the first block of the last reward in batchReward() with no comments. This may lead to difficulty in future maintenance.

     
     function batchReward(...){
     	...
     	rewardedBlocks_[firstBlock] = 1;
     	...
     }

   • Exploit Scenario

     N/A

   • Recommendations

     Use a constant variable to represent the magic number.

   • Results

     Resolved in commit 2600ff8.

7. Naming issues

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol#L10-12；
   Commit	3578295;
   Status	Resolved in commit 0553932.

   • Description

     Distinguishing FIRST_REWARDED_BLOCKand LAST_REWARDED_BLOCK in terms of value and name is redundant since the code logic defined in manualReward() does not check their specific value. They are only used to flag blocks and can be replaced with a single constant variable.

     /// @dev sentinel to mark the start of a range iin rewardedBlocks
     uint constant FIRST_REWARDED_BLOCK = 1;
     /// @dev sentinel to mark the end of a range iin rewardedBlocks
     uint constant LAST_REWARDED_BLOCK = 2;

     In manualReward() function, manualRewardedBlocks_[winner][firstBlock] and manualRewardedBlocks_[winner][lastBlock] are flagged with FIRST_REWARDED_BLOCKand LAST_REWARDED_BLOCK respectively to prevent rewards from being distributed twice. FIRST_REWARDED_BLOCKand LAST_REWARDED_BLOCK functions the same. They are just used to flag blocks. A single non-zero variable can be used to replace these two constant variables.

   • Exploit Scenario

     N/A

   • Recommendations

     These two constant variables can be replaced with a single variable to flag the block.

   • Results

     Resolved in commit 0553932.

8. Use initializer modifier to protect the initialize function

   Severity	Informational
   File	contracts/ethereum/contracts/Token.sol; contracts/ethereum/contracts/AaveLiquidityProvider.sol; contracts/ethereum/contracts/CompoundLiquidityProvider.sol;
   Commit	26e9450;
   Status	Acknowledged.

   • Description

     Token contract, AaveLiquidityProvider contract and CompoundLiquidityProvider contract will be upgradable contracts deployed with beacon proxy. The initializer modifier can be added to the initialize function init() to protect it.

   • Exploit Scenario

     N/A.

   • Recommendations

     Use the Openzeppelin’s initializer from the initializable contract to protect the init() function.

   • Results

     Acknowledged.

     Reply from Fluidity team:

     we use a version count now for dealing with migrations and the like, so the OZ initializer library isn’t really applicable now

9. Lack of Re-entrancy Guard for critical function with external dependency

   Severity	Informational
   Source	contracts/ethereum/contracts/Token.sol;
   Commit	26e9450;
   Status	Pending

   • Description

     The snippets are following a proper check-effects-interaction pattern. However, attention should be paid to the external erc20In and erc20Outthat interacted.

   • Exploit Scenario

     N/A

   • Recommendations

     ReentrancyGaurd can be added to erc20In and erc20Out that contain interactions with the external token transfer.

   • Results

     Pending

1. Unclear usage of blockedRewards_

   Severity	Undetermined
   File	contracts/ethereum/contracts/Token.sol#L107;
   Commit	26e9450;
   Status	Acknowledged.

   • Description

     The blockedRewards_ is used to store users’ quarantine rewards. However, the protocol only updates the value of blockedRewards_ and never uses it to mint rewards.

   • Exploit Scenario

     N/A.

   • Recommendations

     N/A.

   • Results

     Reply from Fluidity team:

     “blockedRewards_ gets used in the unblockReward function, which allows the operator to unblock and mint a reward after external verification.”

There are three major privileged roles for the token contract:

• operator_:

  A multi-sig wallet controlled by the Fluidity dev team that can call below functions.

  • setRestrictions()
    • update maxUncheckedReward_, mintLimitsEnabled_, remainingGlobalMint_, userMintLimit_ and userMintResetBlock_ variables.

  • updateOperator()
    • update Operator multi-sig wallet.

  • enableEmergencyMode()
    • enable emergency mode.

  • disableEmergencyMode()
    • disable emergency mode.

  • updateWorkerConfig()
    • update worker configure which works as a register for the protocol.

  • updateRewardQuarantineThreshold()
    • update RewardQuarantineThreshold.

  • enableMintLimits()
    • enables or disables mint limits.

  • unblockReward()
    • unblocks a reward that was quarantined for being too large.

• emergencyCouncil_:

  • A multi-sig wallet controlled by the Fluidity dev team that can trigger the emergency mode.

• oracle:

  • Distribute rewards by calling function batchReward() or signing reward messages off-chain (signature verified and rewards distributed by function manualReward()).

• operator_:

  A multi-sig wallet controlled by the Fluidity dev team.

  • updateOracles()
    • update token contract oracles

  • enableEmergencyMode() and disableEmergencyMode()
    • enable or disable Emergency Mode.

• emergencyCouncil_:

  • A multi-sig wallet controlled by the Fluidity dev team that can trigger the emergency mode.