Reach out for an audit or to learn more about Macro
or Message on Telegram

Infinex A-3

Security Audit

May 24, 2024

Version 1.0.0

Presented by 0xMacro

Table of Contents

Introduction

This document includes the results of the security audit for Infinex's smart contract code as found in the section titled ‘Source Code’. The security audit was performed by the Macro security team from April 8 to April 18, 2024. Additionally, smaller changes were reviewed on June 21st, 2024.

The purpose of this audit is to review the source code of certain Infinex Solidity contracts, and provide feedback on the design, architecture, and quality of the source code with an emphasis on validating the correctness and security of the software in its entirety.

Disclaimer: While Macro’s review is comprehensive and has surfaced some changes that should be made to the source code, this audit should not solely be relied upon for security, as no single audit is guaranteed to catch all possible bugs.

Overall Assessment

The following is an aggregation of issues found by the Macro Audit team:

Severity Count Acknowledged Won't Do Addressed
High 4 - - 4
Medium 5 3 1 1
Code Quality 2 - - 2
Informational 3 - - 1

Infinex was quick to respond to these issues.

Specification

Our understanding of the specification was based on the following sources:

Trust Model, Assumptions, and Accepted Risks (TMAAR)

Entities:

Safes:

The following are multi-sig smart contract wallets with varying councils set that determine it’s owners, and varying thresholds. Each veto signer set is required to sign of each transaction, or it cannot execute.

Councils:

IRMigrator:

Contract with the ability to change the councils of each safe wallet, and change the members of each council. Intended to be used to allow adjusting aspects of governance if required. Controlled by the deployment safe.

Assumptions:

Source Code

The following source code was reviewed during the audit:

Specifically, we audited the following contracts within this repository.

Note: Currently the referenced repository is private, but there are plans to make it public in the future.

Contract SHA256
src/direct-council/SingleCouncillorNft.sol

60c948e36d2927489316b7d174edfc077fea5934007a9f1dfb58598b2d24127d

src/direct-council/modules/DirectElectionModule.sol

528c9bb72914e65644138a0b945a794a3836ef6854c38ada68bd4d32175f4d26

src/direct-council/storage/DirectElectionStorage.sol

e696cf54e719686f12e677a49ab43e4c8bc8614aa716d82592c47343e27063bc

src/operations-council/modules/OperationsElectionModule.sol

16d3a3a09685997934fd2b750be29a5f269db71b1a433e135bdeced7d0fe02ad

src/security-council/modules/SecurityElectionModule.sol

81ab20948a8de1bd2b4b33cd58086ab1903599d7f34a318a7f15e13b13eca00b

src/treasury-council/modules/TreasuryElectionModule.sol

ff5d20a40d1e4823dce9e58ae5412349adffa2c8f6bfa3a6b64545b671d20456

src/ecosystem-council/modules/EcosystemElectionModule.sol

2ee0d9c7e2145effd81e2310a806603f1c8c1341ac231b850b4e30c932f1719c

src/ecosystem-council/modules/ElectionVotesMixing.sol

08933308a26dc126831340d244186a1b107f137e5165bfa239ddaa31a4a29d51

src/ecosystem-council/modules/WormholeDeclareAndCastModule.sol

3afe20e77752fd2d9234e7a157afda0b077666ab377d4c87a07695e812dab035

src/ecosystem-council/storage/WormholeDeclareAndCastStorage.sol

b4645be71448415fb0326ea00821c8e3b5ad696ee7bed9c505b98d7412961263

src/trader-council/modules/TraderElectionModule.sol

bf6a03fda8d600bba2f8a3472b25b5bbc2069562b3dd15c9633dc970cb52900e

src/modules/ElectionInspectorModule.sol

7b033debfd518635dafb19feba1db4c3da9db75ed76a4aab1d29e2af9edd4b9e

src/modules/ElectionModule.sol

525774a4f5225c5be1e547ee8dbfb4e75d6ceded2097af3cf8b91cb737e8c695

src/modules/MigratorModule.sol

de05d3a906cfbb31f52a23daae37e0746e6fb30995af343c35ef1a4d5fcfee10

src/modules/OwnerModule.sol

bdc76223d20d93cd0e925fb28a80526ada9758885c6b0bab3af6a7bf213ae1fe

src/modules/RenounceableOwnerModule.sol

5e0db23c09ba88758b44aa8dc8ac2915e38a03533194e894c157819bcfb167e5

src/governance-points/GovernancePointsAdapter.sol

f7072cc28bb43e998d56cebbbef211bfb0c330dbe2812abe7755a4c0ebd5e4a9

src/ir-migrator/IRMigrator.sol

8fcb39f92c80e0ab2e5dfe493a03f9a44cdb6f54db612e48464818b500e850ed

src/safe-module/BaseSafeModule.sol

411c20a38ae3e02de375abea4b7b72bf653237d2b6bf3774ebdd96b8bb1b6be9

src/safe-module/InfinexSafeModule.sol

6ff75ebcd5350fa2aee2df197a68dcc8a6459e3136d7fb5825e746f270ac9bbd

src/safe-module/ReceiverSafeModule.sol

0833dd9310f4116a88f722c7e7982f89dd7ba3564d69da0365ddb7a3725a5bc4

src/safe-module/SafeModuleRegistration.sol

0669bad9176a58227b3215f1d8fa9f7f4ffa868087b0d45d174b75cca4f4f3f2

src/storage/AdminStorage.sol

2ca3c4f92ef932af1d9d027dae5e7cd4795f9fbb25e7f4be3ef05135e1c26181

src/storage/MigratorStorage.sol

df373290e2f0a852addf2a1bbd8fe536fde112279b9faab20871bd3bd8f9a27d

Note: This document contains an audit solely of the Solidity contracts listed above. Specifically, the audit pertains only to the contracts themselves, and does not pertain to any other programs or scripts, including deployment scripts.

Issue Descriptions and Recommendations

Click on an issue to jump to it, or scroll down to see them all.

Security Level Reference

We quantify issues in three parts:

  1. The high/medium/low/spec-breaking impact of the issue:
    • How bad things can get (for a vulnerability)
    • The significance of an improvement (for a code quality issue)
    • The amount of gas saved (for a gas optimization)
  2. The high/medium/low likelihood of the issue:
    • How likely is the issue to occur (for a vulnerability)
  3. The overall critical/high/medium/low severity of the issue.

This third part – the severity level – is a summary of how much consideration the client should give to fixing the issue. We assign severity according to the table of guidelines below:

Severity Description
(C-x)
Critical

We recommend the client must fix the issue, no matter what, because not fixing would mean significant funds/assets WILL be lost.

(H-x)
High

We recommend the client must address the issue, no matter what, because not fixing would be very bad, or some funds/assets will be lost, or the code’s behavior is against the provided spec.

(M-x)
Medium

We recommend the client to seriously consider fixing the issue, as the implications of not fixing the issue are severe enough to impact the project significantly, albiet not in an existential manner.

(L-x)
Low

The risk is small, unlikely, or may not relevant to the project in a meaningful way.

Whether or not the project wants to develop a fix is up to the goals and needs of the project.

(Q-x)
Code Quality

The issue identified does not pose any obvious risk, but fixing could improve overall code quality, on-chain composability, developer ergonomics, or even certain aspects of protocol design.

(I-x)
Informational

Warnings and things to keep in mind when operating the protocol. No immediate action required.

(G-x)
Gas Optimizations

The presented optimization suggestion would save an amount of gas significant enough, in our opinion, to be worth the development cost of implementing it.

Issue Details

H-1

Veto signers can be spoofed

Topic
Spoofing
Status
Impact
High
Likelihood
Medium

BaseSafeModule.sol is a module and guard for multiple safe wallets that control the infinex protocol. Depending on how they are setup, there are two types of signers set, council and veto signers. In order for the safe to execute calls, it checks with the BaseSafeModule via the checkSignatures function, and requires council signers need to reach a quorum of half of the total signers rounded up, while all veto signers are required to sign on. The safe initially verifies that each signer is an owner, and there are no duplicates, before it call the guard. Because the safe already checked for duplicates, it assumes that there cannot be and forgoes the same check.

  ...
  // it doesn't make sense to check for signers order or address 0 because those checks are performed on the
  // Safe with checkSignatures before calling this function
  ...

Reference: BaseSafeModule.sol#L167-L168

However, in the case where v == 1 the signer is assumed to be the msg.sender, but in the safe, v == 1 can also mean the signer pre approved the call via the approveTransaction call. This means that the signer verified in the safe can differ than the one verified in the guard when the pre-approved signer ≠ msg.sender.

} else if (v == 1) {
    // v ==1 means that the sender is approving the txn, or its an approvedHash (which we are not going to
    // deal with here)
    curOwner = msgSender;
}

Reference: BaseSafeModule.sol#L152-L156

else if (v == 1) {
    // If v is 1 then it is an approved hash
    // When handling approved hashes the address of the approver is encoded into r
    currentOwner = address(uint160(uint256(r)));
    // Hashes are automatically approved by the sender of the message or when they have been pre-approved via a separate transaction
    if (executor != currentOwner && approvedHashes[currentOwner][dataHash] == 0) revertWithError("GS025");
}

Reference: Safe.sol#L297-L303

In the case where there are at least 2 veto signers, but one does not want to sign for the transaction, and there are more council signers in favour of the transaction than required, then the veto members can be spoofed by the extra members by doing the following:

  • extra council members call approveHash() on the safe to pre-approve the transaction
  • for the signatures of pre-approved council members, set the signatures v value to 1 and encode signers address in the signatures r value.
  • have the one veto member that is in favour initiate the call to execTransaction(), which will cause them to be counted an additional time for each pre-approved council member.

Remediations to Consider

Use the same address as the safe uses for the signer, and/or check to make sure there are no duplicate signers as is done initially in the safe.

H-2

Duplicate council members prevent updating the Safe owners

Topic
Protocol Design
Status
Impact
High
Likelihood
Medium

The Infinex’s election system allows anyone to nominate any address for election. During the voting period, users can vote with their assigned voting power on nominated addresses. Once the voting period is over and council members have been elected, calling resetSafeSigners on the InfinexSafeModule updates the Safe owners according to the elected council members.

As multiple councils - each with its own election system - are used within the Infinex protocol, there is nothing to prevent an address getting elected on different councils simultaneously. If this happens, a call to resetSafeSigners would revert, preventing the Safe owners to get updated.

Consider the following scenario:

  • The Ecosystem council elects member 0x123.
  • Similarly, the same address 0x123 gets elected during the Trader Council election.

Since each of the four Safe wallets (TreasurySafe, DeployerSafe, OperationsSafe, and SecuritySafe) is associated with both the Ecosystem Council as well as the Trader Council, updating Safe owners via resetSafeSigners would include 0x123 twice into the councilSigners array and trying to add it twice as an owner to the Safe via the addOwnerWithThreshold call here. However, the Safe reverts when an address is added that is already configured as an owner:

// No duplicate owners allowed.
if (owners[owner] != address(0)) revertWithError("GS204");

Reference: OwnerManager.sol#L54C1-L55C67

As a result, the new elected council members cannot be applied as signers to the different Safes, leaving the old owners in the Safes. The old owners remain signers until new council members get elected (without duplicates) or an appropriate proposal is executed through the migrator to remove or add certain council members.

Remediations to Consider

Handling duplicate members across different councils can be addressed in different parts of the protocol, each with its own pros and cons:

  1. In the resetSafeSigners function, a check can be included to prevent adding an already existing owner to the Safe. In this case, additional logic is required to calculate the requiredSigners amount accordingly. Note that this check is included for veto signers here, but currently left out for council signers.

    Handling duplicate owners this way can lead to scenarios where only a single signer or an undesired small number of signers control a Safe.

  2. A broader redesign of the system could be considered avoiding duplicates in the first place, such as having only one council associated to a Safe.

H-3

Required number of signers not met when veto signer is a council signer, preventing to update the safe owners

Topic
Protocol Design
Status
Impact
High
Likelihood
Medium

When updating the Safe owners via resetSafeSigners on the InfinexSafeModule, the transaction may revert if the required signers threshold is not met. In particular, this happens when a veto signer is already among the council signers.

Consider the following case:

  • Given Council_A with 2 signers → councilThreshold = 2
  • Given Council_B with 1 veto signer → vetoSignersThreshold = 1
  • This results in requiredSigners = 3

This works well as long as all 3 signers are unique, meaning that the 1 veto signer of Council_B isn’t one of the signers of Council_A.

In the case a veto signer is one of the signers of Council_A, adding the veto signer to the Safe will be skipped due to the else if check below:

for (uint256 i = 0; i < vetoSigners.length; i++) {
    if (oldSigners.length > 0 && vetoSigners[i] == oldSigners[0]) {
        oldSigners = new address[](0);
        ++addedSigners;
    **} else if (!targetSafe.isOwner(vetoSigners[i])) {**
        _execOnSafe(
            targetSafe,
            abi.encodeWithSelector(
                ISafe.addOwnerWithThreshold.selector,
                vetoSigners[i],
                requiredSigners < ++addedSigners ? requiredSigners : addedSigners
            )
        );
    }
}

Reference: BaseSafeModule.sol#L71

Up to this point, it works just fine and we have a calculated requiredSigners = 3 and a addedSigners = 2. In the final step of the _resetSafeSigners function, the old signer is removed:

if (oldSigners.length > 0) {
    address prevOwner = councilSigners.length > 0 ? councilSigners[0] : vetoSigners[0];
    _execOnSafe(
        targetSafe,
        abi.encodeWithSelector(ISafe.removeOwner.selector, prevOwner, oldSigners[0], requiredSigners)
    );
}

Reference: BaseSafeModule.sol#L83-L89

This is where the transaction fails, as we attempt to set the required signers threshold to 3, but only two signers have been added to the Safe.

As a consequence, similar to H-2, the new elected members cannot be applied.

Remediation to Consider

In the final step when removing the old signer, the threshold needs to be set to either requiredSigners or addedSigners, whatever is smaller.

H-4

Ownership of IRMigrator can be taken over after deployment

Topic
Frontrunning
Status
Impact
High
Likelihood
Medium

Deployment of the Infinex Governance contracts is done by using the Cannon tool. Throughout the various deployment steps, the contracts are usually initialized with an owner set to the deployer admin and after the Safes are deployed, a new owner is nominated corresponding the new deployed Safe address. Let’s look at the deployment steps for SecurityElectionModule:

  1. SecurityElectionModule is deployed together with OwnerModule using the Router setup:

    [router.SecurityElectionRouter]
    contracts = [
      "OwnerModule",
      "SecurityElectionModule",
    ]
    
  2. Owner is nominated to deployer admin

    [invoke.SecurityElectionRouterNominate]
    target = ["SecurityElectionRouter"]
    func = "nominateNewOwner"
    args = ["<%= settings.admin %>"]
    depends = ["router.SecurityElectionRouter"]
    
  3. Deployer admin accepts ownership

    [invoke.SecurityElectionRouterAccept]
    target = ["SecurityElectionRouter"]
    func = "acceptOwnership"
    depends = ["invoke.SecurityElectionRouterNominate"]
    
  4. Now, the module can be initialized with initializeElectionModule

    [invoke.SecurityElectionRouterInit]
    target = ["SecurityElectionRouter"]
    func = "initializeElectionModule"
    args = [
      "<%= settings.SEC_TOKEN_NAME %>",
      "<%= settings.SEC_TOKEN_SYMBOL %>",
      "<%= settings.initialSecurityCouncillor %>",
    ]
    
  5. Finally, after SecuritySafe has been deployed, the Safe’s address can be nominated to become the new owner

    # Security Safe will have to call acceptOwnership, so the initial councillors will have their first job
    [invoke.SecurityElectionSafeOwner]
    target = ["SecurityElectionRouter"]
    func = "nominateNewOwner"
    args = [
      "<%= imports.security_council_safe.contracts.Safe.address %>"
    ]
    depends = ["provision.security_council_safe"]
    

After deployment, the owner of the SecurityElectionModule is still set to the deployer admin until the SecuritySafe accepts the ownership.

The above deployment steps for the SecurityElectionModule are fine (apart from the risk mentioned in X-?) and after deployment, ownership can only be taken over by the SecuritySafe.

However, this is not the case for the IRMigrator contract. Let’s look at how this contract is deployed:

  1. IRMigrator is deployed

    [contract.IRMigrator]
    artifact = "IRMigrator"
    args = [
      "<%= settings.admin %>", # This is changed to DeployerSafe later
      "<%= settings.crossDomainMessenger %>",
      "<%= settings.wormholeRelayer %>",
      "<%= settings.OP_WORMHOLE_SOURCE_CHAIN %>"
    ]
    

    and passes the deployer admin (settings.admin) as first argument to the constructor, which is nominated as the new owner:

    constructor(address _owner, address _crossDomainMessenger, address _wormholeRelayer, uint16 _sourceChain) {
        **nominateNewOwner(_owner);**
        _setCrossDomainMessenger(_crossDomainMessenger);
        _setWormholeRelayer(_wormholeRelayer);
        _setSourceChain(_sourceChain);
    }
    

    However, it has to be noticed that the ownership is not accepted in the constructor above.

  2. Finally, after DeployerSafe has been deployed, the Safe’s address is nominated to become the new owner:

    # DeployerSafe will have to call acceptOwnership, so the initial councillors will have their first job
    [invoke.IRMigratorElectionAdminSet]
    target = ["IRMigrator"]
    func = "nominateNewOwner"
    args = [
      "<%= imports.deployer_safe.contracts.Safe.address %>"
    ]
    depends = ["provision.deployer_safe"]
    

After deployment, the IRMigrator contract remains without an owner and requires the DeployerSafe to accept the ownership, which may take a while to get approval from all required signers.

Since there is no owner initialized until this point, anyone can call nominateNewOwner with their own address and a following call to acceptOwnership to take over the ownership. As the owner of the IRMigrator, someone can take full control over the entire Infinex Governance by letting proposals pass (and setting the merkle tree accordingly) such as adding and removing certain members from the councils.

Remediation to Consider

The owner of the IRMigrator needs to be initialized during deployment. This is can be achieved by adding a call to acceptOwnership in the constructor after the owner is nominated.

M-1

Owners can gain permanent control over the Safe

Topic
Trust Model
Status
Wont Do
Impact
High
Likelihood
Low

During the deployment of Safe wallets, a Safe module as well as a guard are configured on the Safe, which are both implemented in InfinexSafeModule.sol. The module provides functionality for updating the Safe owners based on the elected council members, while the guard’s responsibility is to verify that the required number of council signers and veto signers have approved the transaction. This is done via the checkTransaction function.

An important part of the Infinex protocol is the rotation of Safe owners after each election period, typically every 90 days, according to the defined election cycle.

However, the guard’s checkTransaction function doesn’t prevent certain transactions from being executed, allowing the current owners to initiate any transaction as long as they reach consensus (meeting the minimum number of signers). This would enable them to initiate a transaction that disables the module or the guard. If this happens, any functions defined in InfinexSafeModule.sol cannot be further executed, involving updating the owners after new council members are elected. As a result, the current owners would retain control over the Safe forever.

Remediations to Consider

Prevent certain functions from getting executed in the guard’s checkTransaction function, including disable a module and removing a guard.

Response by Infinex

Finding an issue in the future for the SafeModule and not being able to replace it could lead to kind off bricking the whole governance. To take advantage of this risk, councillors would need to collude. This is considered as an accepted risk.

M-2

An invalid councillor can prevent Safes from getting updated

Topic
Validation
Status
Impact
High
Likelihood
Low

In the Operations and Security council, a councillor can be set directly by the owned Safe via setCouncillor. All Safes associated with one of those DirectElection councils, the councillor is set to one of the Safe owners when calling resetSafeSigners on the InfinexSafeModul.

When addOwnerWithThreshold is getting called inside the resetSafeSigners function, the Safe validates the passed address as follows:

function addOwnerWithThreshold(address owner, uint256 _threshold) public override authorized {
    // Owner address cannot be null, the sentinel or the Safe itself.
    if (owner == address(0) || owner == SENTINEL_OWNERS || owner == address(this)) revertWithError("GS203");

Reference: OwnerManager.sol#L53

This means the owner can only be added successfully if the address is not null, the sentinel, or the Safe itself.

However, there is no appropriate address validation done in the setCouncillor function, allowing the councillor be be set to any of these invalid addresses. Consequently, resetting the safe signers would revert, and the current owners would retain control over the Safe until a valid councillor address is added. In addition, there is the risk of adding a councillor’s address that is an elected member of a different council (as stated in H-2), also causing resetSafeSigners to fail.

Setting an invalid address could be done on purpose by a required group of signers to avoid being replaced as Safe owners by new election members, or could happen unintentionally.

Remediation to Consider

The setCouncillor functions should prevent setting an invalid councillor address. Adding checks to not allow address(0) and the sentinel address is straight forward. Disallowing any of the Safes addresses would require additional logic to reference those in the DirectMigratorModule directly.

M-3

The deployer safe has the most power, but least security

Topic
Trust Model
Status
Acknowledged
Impact
High
Likelihood
Low

Deployer safe is intended to control the IRMigrator contract, which itself has the power to change the councils that are signers for other safes, as well as directly change the members of these councils. This gives the IRMigrator contract power to override the other contracts involved in governance, which is by design as it is intended to allows governance to be flexible, but means it has the most control over governance structure, and since the deployer safe owns and control this, it would be expected to have the highest quorum required, and additional guards such as veto signers. However, based on the cannon deployment script, it has the least quorum requirements with a threshold of 4 but the most potential signers of 7, and no veto signer requirement. The most strict requirement for the safes is the treasury safe, which has a threshold of 5, with 7 signers, with the treasury council signer being required as a veto signer, but has the same councils as with the deployer.

In the case where a quorum of council members, 4 in this case, were malicious and there goal was to steal assets owned by the treasury safe, they wouldn’t have the signatures to directly execute on the safe, however they could steal these assets by using their control of the deployer safe if the following way:

  • Create a proposal on the IRMigrator contract to adjust the councils that own the treasury safe to councils that they control, resulting in them gaining control of the safe.
  • Have the deployer safe set a malicious merkle root tied to this new proposal, where the leafs are only for addresses of these malicious council members, giving them control over voting on this proposal.
  • Vote to pass the proposal.
  • Wait 7 days for the proposal to complete the voting period, and not sign treasury safe requests to move the funds out in response to the pending proposal, keeping the funds locked.
  • Execute the proposal, then with the new owners of the safe drain the funds.

The above example shows that the deployer safe in its current state should be the hardest for signers to corrupt, as it can otherwise lead to control of more secure safes.

Remediations to Consider

Increase the threshold of signers for the deployment safe, or have a council set as a veto signer. The end result should be that it has the most requirements compared to the other safes, or at least tied with the most requirements.

M-4

Deployer safe take over governance with a malicious root

Topic
Trust Model
Status
Acknowledged
Impact
High
Likelihood
Low

The deployer safe is intended to have the ability to set the merkle root associated with determining voting power for both the IRMigrator and the Ecosystem Council. This root is trusted to be set to correctly relate to snx token balances across multiple chains, but there is no certainty that this root is correct. As mentioned in issue M-3, a malicious root can be set which can lead to controlling governance, or also effect the results of a election for the Ecosystem Council.

Considering how much control the root has over Infinex governance, there should be consideration into ways to verify the root. Although verifying a tree on-chain is potentially expensive, and difficult considering balances over multiple chains need to be considered, off-chain verification is possible, using services like chainlink functions or keepers, or other trusted services.

Remediations to Consider

Set up a method to verify the merkle tree, likely off chain, and have this be a veto signer for the deployer safe, to ensure that any merkle root has been verified to accurately relate to the voting power expected for a given council or the IRMigrator.

M-5

Deployer can set a malicious bridge contracts

Topic
Trust Model
Status
Acknowledged
Impact
High
Likelihood
Low

As mentioned is issue M-4, and M-3, the deployer safe can control governance using a malicious root. However, if this is resolved and verified as suggested in M-4, another method for control remains. The deployer safe can also set the addresses of the contracts responsible for bridging in the IRMigrator, these bridge addresses are trusted that provided sender of the message is correct and from a target chain. However, this bridge address can be set to anything and provide potentially any address, which the IRMigrater will use to generate the leaf and verify the provided proof to determine voting power and cast a vote. If the merkle tree is correct, a malicious bridge address could be used to spoof snx token holders and allow votes to be made on malicious proposals.

Remediations to Consider

Set the associated bridge addresses to be immutable, preventing them from being adjusted maliciously. It is important to note that in doing so it would prevent responding to the potential of the set bridge no longer functioning for various reasons, and may require redeploying the governance contracts if this does occur and the bridge address needs to be changed.

Q-1

Replace synthetix with infinex for storage slot generation

Topic
Best Practice
Status
Quality Impact
Low

Some contracts use the unstructured storage pattern, where storage data is separated into related parts as structs at a storage slot that is typically a hash of a unique string defining it’s purpose. This ensures that there is no storage collisions unless that same string is used for other data within the same contract. An example is the DirectElectionStorage contract which stores data required by the DirectElectionModule, as described in the comment showing the hash used to generate the storage slot:

contract DirectElectionStorage {
  struct DirectElectionStore {
      // True if initializeElectionModule was called
      bool initialized;
      // The address of the council NFT
      address councilToken;
  }

  function _directElectionStore() internal pure returns (DirectElectionStore storage store) {
      // solhint-disable-next-line no-inline-assembly
      assembly {
              //@audit: notice the slot is the hash of the s
          // bytes32(uint(keccak256("io.synthetix.directelectionmodule")) - 1)
          store.slot := 0x7c60e870ca22a961b6f9ac5a5e422b050949150c6dcb0e0682dc5a3d0c8c0024
      }
  }
}

However, the hash used for this and other similar storage contracts use “synthetix” in the string, which these contracts are pulled from, but these should be changed to “infinex” as this is for the infinex protocol.

Remediations to Consider

Change the storage slot hashes to use infinex rather than synthetix.

Q-2

Nitpicks

Topic
Nitpicks
Quality Impact
Low

The following are just typos in comments:

I-1

Governance deployments can be front-ran and griefed

Topic
Deployment
Status
Impact
Informational

These governance contracts are intended to be deployed using Cannon, which will deploy these contracts following a cannonfile script, like this one for base. It is setup to execute the defined deployments or function calls, where some calls are dependent on another executing. For example these two calls in the base cannonfile:

[invoke.SecurityElectionRouterNominate]
target = ["SecurityElectionRouter"]
func = "nominateNewOwner"
args = ["<%= settings.admin %>"]
depends = ["router.SecurityElectionRouter"]

[invoke.SecurityElectionRouterAccept]
target = ["SecurityElectionRouter"]
func = "acceptOwnership"
depends = ["invoke.SecurityElectionRouterNominate"]

Reference: [cannonfile.base.toml#L136-L145]([invoke.SecurityElectionRouterNominate] target = ["SecurityElectionRouter"] func = "nominateNewOwner" args = ["<%= settings.admin %>"] depends = ["router.SecurityElectionRouter"] [invoke.SecurityElectionRouterAccept] target = ["SecurityElectionRouter"] func = "acceptOwnership" depends = ["invoke.SecurityElectionRouterNominate"])

In this case a call to nominateNewOwner() is made to the SecurityElectionRouter, nominating the admin, and it depends on the SecurityElectionRouter to have been deployed. Then the next call is for the admin to accept ownership, and will only execute after the prior nomination call has executed.

However, these are executed in separate transactions, and allow someone to front run the acceptance of ownership, as mentioned in issue H-4, since it is currently unowned anyone can nominate a new owner, and take ownership. In this case the nomination and/or acceptance needs to occur before ownership is accepted by the admin.

This is one case of possible front-running during deployment, multiple exist, like for each initialization call after deployment, where there are no permission requirements.

Any front running would be caught during deployment as some functions would fail, and if this occurs redeployment is likely necessary. Make sure to check the state of these contracts are as intended after deployment, and/or consider setting up factory contracts that handle deployment, initialization and setting ownership in a singular transaction, but this may disrupt the intended cannon deployment flow.

Response by Infinex

The fix includes changes on the way the ownership is initialized for the ElectionModule routers on the deployment scripts by using initializeOwnerModule instead of nominateNewOwner and acceptOwnership. This partially address the issue.

I-2

Splitting snx balance across accounts increases voting power

Topic
Voting Power
Impact
Informational

Synthetix debt share in the form of snx tokens are used to determine voting power for the ecosystem council and IRMigrator. Voting power is determined by taking the square root of the users snx balance, which has the effect of reducing the voting power of larger holders relative to smaller ones, since the square root reduces larger numbers substantially more than smaller numbers. However, this effect on large holders of snx tokens could be circumvented, by splitting up their token balance across multiple accounts, reducing the loss of voting power. This may give an unfair boost in voting power to large holders that decide to do this, relative to holders that maintain a large balance in a single account. Typically it would be assumed that splitting balances across many accounts would be costly and cumbersome, but if there is an incentive to do so, potentially by supporting malicious proposals, then malicious actors may end up with more sway than honest whales.

Response by Infinex

This problem is inherent to quadratic voting.

I-3

Incentive misalignment of snx voters

Topic
Voting Power
Impact
Informational

As mentioned Synthetix snx tokens are used for voting in both the ecosystem council and the IRMigrator contract, which gives these holders a lot of control over infinex governance. Synthetix governance also gives voting power to these users, but since these tokens are directly tied to the synthetix protocol, larger holders are incentivized to vote honestly for proposals that benefit Synthetix as a whole. This is not so much the case for voting on infinex proposals as there is no direct link between snx token value and the wellbeing of Infinex. This lack of incentive alignment could lead to higher risk of malicious proposals and votes, and should be observed with additional caution.

Disclaimer

Macro makes no warranties, either express, implied, statutory, or otherwise, with respect to the services or deliverables provided in this report, and Macro specifically disclaims all implied warranties of merchantability, fitness for a particular purpose, noninfringement and those arising from a course of dealing, usage or trade with respect thereto, and all such warranties are hereby excluded to the fullest extent permitted by law.

Macro will not be liable for any lost profits, business, contracts, revenue, goodwill, production, anticipated savings, loss of data, or costs of procurement of substitute goods or services or for any claim or demand by any other party. In no event will Macro be liable for consequential, incidental, special, indirect, or exemplary damages arising out of this agreement or any work statement, however caused and (to the fullest extent permitted by law) under any theory of liability (including negligence), even if Macro has been advised of the possibility of such damages.

The scope of this report and review is limited to a review of only the code presented by the Infinex team and only the source code Macro notes as being within the scope of Macro’s review within this report. This report does not include an audit of the deployment scripts used to deploy the Solidity contracts in the repository corresponding to this audit. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. In this report you may through hypertext or other computer links, gain access to websites operated by persons other than Macro. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such websites’ owners. You agree that Macro is not responsible for the content or operation of such websites, and that Macro shall have no liability to your or any other person or entity for the use of third party websites. Macro assumes no responsibility for the use of third party software and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.