2023-11-11 Audit Report - Neutron - Duality v0.5.0 + integration of Cosmos SDK 0.47

Security Audit Report

Neutron: Duality v0.5.0 + integration of Cosmos SDK 0.47

Authors: Ivan Gavran, Andrey Kuprianov, Nikola Jovicevic

Table of Contents Audit overview............................................................................................................. 1 The Project 1 Conclusions 1 Audit dashboard.......................................................................................................... 3 Target Summary 3 Engagement Summary 3 Severity Summary 3 System Overview and Checked Threats .................................................................... 4 Duality v0.5.0 4 Duality upgrade to Cosmos SDK v0.47.3 4 Axelar GMP middleware 5 Refactoring of cacheCtx logic in Transfer and InterchainTxs modules 5 Updates to x/adminmodule 6 Fork of the Cosmos SDK slashing module 7 Backport of bank-hooks from Osmosis 8 Findings ....................................................................................................................... 9 Unbounded iteration over Incentives stakes may lead to chain halt 11 Stealing of user funds via rounding errors in pool and limit order swaps 13 Stealing of arbitrary funds via IBC swaps 16 Quadratic scaling for multi-amount deposits may lead to DOS 20 Contract failures potentially remaining in the state indefinitely 24 Misleading Incentives module user-facing and developer documentation 26 Limit order tranche security rests on a fine interplay of implicit assumptions 28 Incomplete validation of MsgMultiHopSwap may lead to user fund loss and frustration 31 Incomplete validation of pool denominations 33 Miscellaneous security concerns 35 Tick and Fee inputs are not completely validated 37 Miscellaneous code improvements 40 Unutilized event collection 42 Error message referring to a wrong proposal id 43 Multi-message proposals stored as different proposals 45 © 2023 Informal Systems Neutron: Duality v0.5.0 + integration of Cosmos SDK 0.47

Duality modules still use deprecated x/params 47 Appendix: Vulnerability Classification ..................................................................... 48 Impact Score 48 Exploitability Score 48 Severity Score 49 Disclaimer.................................................................................................................. 51 © 2023 Informal Systems Neutron: Duality v0.5.0 + integration of Cosmos SDK 0.47

Audit overview The Project In October 2023, Informal Systems has conducted a security audit for Neutron. The audit covered the following areas:

Duality v0.5.0, focusing on changes since the last audit at the version v0.2.0
Duality upgrade to Cosmos SDK v0.47.3, introduced in the PR#423
Integration of Axelar GMP middleware (x/gmp module at commit 9a208a52) into Duality v0.5.0.
Neutron update to Cosmos SDK v0.47, which includes the refactoring of the cachedCtx logic, as well as the implementation of parameters handled by each module separately. These changes required checking all the modules of the neutron repository, especially x/contractmanager , x/interchaintxs and x/ transfer due to the refactored logic. Commit hash was 261f47c3.
Updates to the admin module. Admin module us used for executing privileged actions, such as making software update proposals. The focus of the audit was on handling of the new admin proposals. The audited repository was admin-module at the commit hash d5cd5ae2.
Fork of the Cosmos SDK slashing module, which improves efficiency of the validator's missed block bitmap, and is a backport from Cosmos SDK v0.50. This implied auditing the slashing module repo at the commit hash efa64930.
Backport of bank-hooks from Osmosis, using TrackBeforeSend and BlockBeforeSend hooks within the tokenfactory module. Additionally, we checked the bank module, in which the hooks are triggered. The audit was performed from October 2, 2023 through October 31, 2023, by the following personnel: • Ivan Gavran • Andrey Kuprianov • Nikola Jovicevic

Conclusions We performed a thorough review of the project, and found it to be implemented carefully and equipped with a suite of integration tests. Nonetheless, some subtle mistakes were found. In our audit, we report the total of 16 findings: three of them of the critical severity, and one of the high severity.

The most crucial necessary improvements are to perform complete validation of Duality input messages, coming both via standard transactions, as well as via IBC: the most severe critical finding, allowing to steal arbitrary amounts of funds, is due to missing validation of inputs. Several other findings, some with undefined consequences, can be prevented when all inputs are properly validated. Another source of concerns seems to be the DOS attacks made possible to scaling the system in various dimensions, such as the number of stakes in the incentives program, or the number of pool shares obtained in the scope of a single transaction. To address this, we recommend to introduce a category of tests that specifically checks the system behavior when scaling various parameters. Furthermore, various mathematical operations, both with the native datatypes as well as with rational numbers, are a source of concern, to which we recommend to pay special attention. Finally, in some cases the unit tests are not kept up to date with the codebase changes, which reduces the overall robustness of the codebase.

Disclaimer wrt. Duality integration into Neutron We should point out that we have audited the Duality codebase as a separate project, at v0.5.0. No integration of Duality into Neutron codebase was in the scope of the present audit. All findings regarding Duality have been acknowledged, and are valid wrt. v0.5.0; but as has been pointed out by Neutron, two findings (Unbounded iteration over Incentives stakes may lead to chain halt and Misleading Incentives module user-facing and developer documentation) don’t apply to the Neutron v1.1.0 release, and have been therefore marked Outdated in this

report. Also all fixes wrt. other Duality findings are PRs against neutron-org/neutron repository, not against the audited duality-labs/duality repository. Informal Systems has not audited integration of Duality into Neutron v1.1.0, and can provide no guarantee of any kind wrt. this integration. Although in general we consider Neutron and Duality code to be of very high quality, we consider it our responsibility to point out that in particular the late, post-audit and pre-release changes are often the reasons of security vulnerabilities. We recommend Neutron to arrange a separate audit of Neutron v1.1.0 release.

Audit dashboard Target Summary • Type: Protocol and Implementation • Platform: Go • Artifacts: admin-module, neutron, cosmos-sdk, duality

Engagement Summary • Dates: 02.10.2023 -- 31.10.2023 • Method: Manual code review, protocol analysis

Severity Summary Finding Severity #

Critical 3

High 1

Medium 0

Low 5

Unknown 2

Informational 5

Total 16

System Overview and Checked Threats Duality v0.5.0 Duality at v0.5.0 is about to be integrated into Neutron. We've been conducting a previous audit of Duality at v0.2.0, thus the present audit is to a large degree based on the previous one, and we highlight only the important differences wrt. v0.2.0. For the overview of the core Duality functionality and its security we refer the reader to the "System Overview" and "Threat Inspection" sections of the Duality v0.2.0 audit report. From the point of view of external system interfaces, the introduction of the IBC Swap module is a very important change, allowing anyone to piggyback a limit order in the memo field of the ICS-20 fungible token transfer IBC packet. The limit order will then be filled, if possible, immediately upon receiving of the ICS-20 packet: only Fill-Or- Kill limit orders are permitted currently. The single most important internal change is that of Introduction of Pool IDs: while in the old version liquidity pools have been identified internally by the combination of pair id, pricing tick and the fee (see deposit_denom.go at v0.2.0), in the new version each liquidity pool is identified by an integer Pool ID (see pool_denom.go at v0.5.0): a new id is introduced if and only if tokens are deposited for a token pair at a specific tick and fee layer. We can highly praise Duality developers for adding this additional level of indirection, as it represents a significant line of defense wrt. possible attacks: it is now not possible to craft the system inputs (ticks and fees) in order to scoop into a different pool than intended.

Duality upgrade to Cosmos SDK v0.47.3 Duality upgraded its codebase to Cosmos SDK v0.47.3 in the PR#423. Our approach to inspecting the upgrade was to check all the changes from the PR and validate their impact. Furthermore, we matched the changes with the SDK’s upgrade document. What made the upgrade easier in this case was that Duality is not a live chain (and thus no live upgrade was necessary) and that Duality’s modules are set to be merged with the Neutron chain. Thus, we directed our attention to changes in Duality’s modules. Overall, we find the upgrade to be executed correctly. The only exception we noticed was that Duality did not migrate its params from the x/params module to be handled by individual modules, as reported here.

The changes introduced are as follows:

• changes in the math module: replacing the member function toDec by NewDecFromInt • replacing references to the tendermint repository by the ones to the cometbft repository • removing legacy Route and RouterQuerier functions from module.go files • removing now unnecessary RandomizedParams function • removing writing events to the parent (a fix for this was introduced in SDK 0.46.6) • replacing references to gogo/protobuf by cosmos/gogoproto • using IBC v7 , which also implied changing the signature of the SendPacket function

Unrelated to upgrades in the SDK version, there are a couple of more small changes in the PR:

• the function GetRewardsEstimate changes signature. • slice as an argument to the Sub function (which accepts variadic arguments) is expanded • re-formatting

Axelar GMP middleware The Axelar GMP middleware is added for purposes of receiving FungibleTokenPacketData packets from the Axelar network. Axelar’s authentication is not included, so it is basically adapting the packet data before passing it further down the stack. If the memo’s payload field is empty, it indicates that the package should not be handled by the GMP middleware. Otherwise, it’s all about assigning the memo field with the payload data before continuing with OnRecvPacket logic.

Refactoring of cacheCtx logic in Transfer and InterchainTxs modules In order to make the experience of relayers and contract developers easier, the way the gas is spent when smart contracts are handling IBC packets through Sudo calls has been refactored.

In a nutshell, every Sudo acknowledgement handler can spend at most SudoCallGasLimit gas. If the contract spends more gas than allowed (or returns an error), the acknowledgement is still marked as processed, and its payload is saved to the store. This allows the contract to get back to it and process successfully by sending a ResubmitFailure message. At the same time, it makes sure that the relayer is compensated fairly and its work is not abused by a smart contract’s arbitrary behavior. In the diagram below, we outlined the steps of the refactored workflow:

1 A workflow of handling the IBC acknowledgement in the transfer module

Inspected Threats In the audit, we inspected the protocol and its implementation, focusing on these particular threats:

Is it possible that an out-of-gas error is raised outside of the wrapped call? Resolution: Not a viable attack. We inspected all handling outside of the wrapped sudo call (e.g., here) and concluded that the relayer always has a predictable way of estimating the gas consumption.
Is it possible that the stored failures remain in memory indefinitely? Resolution: This is indeed a possibility, as discussed here. The severity is limited due to high-cost of

executing the attack (either paying for multiple IBC calls, or establishing an adversarial relayer to pocket the cost of IBC calls). 3. Is it possible to store the same failed acknowledgement under multiple ContractFailure s? Resolution: Not a viable attack. The reason is that the iterated handling of the acknowledgement will take the error returned from the Sudo call (here, the return is naked) and propagate it (here, here, and here), all until the wasmd ’s msg_dispatcher, which will then ignore any state changes (see here), and thus won’t store this new instance of the ContractFailure . For comparison, when calling the same Sudo call from the transfer module, the error from Sudo is ignored. (And the same is true for the interchaintx module here.) 4. Sudo routing: could a relayer and a malicious contract steal locked funds of another contract? Concretely, could it happen that contract A locks its funds for TimeoutFee and RecvFee , but the unused part of it (eg, TimeoutFee in case of receiving acknowledgement) would be returned to some other contract B ? Resolution: No. The sender contract (to which funds need to be returned) is stored when locking fees under the PacketId key. Thus, a malicious relayer cannot manipulate that receiver, since packetID is derived from ( packet.SourcePort , packet.SourceChannel , packet.Sequence) , whose correctness is verified by the light client for the destination chain.

Updates to x/adminmodule The adminmodule has been refactored to allow for handling more general gov proposals, which do not limit proposals to a pre-defined set, but rather allow it to be a sequence of (whitelisted) messages. At the same time, it still needs to support legacy proposals. For a proposal that has passed on the contract side, its messages get executed within the DispatchMsg function as either legacy or non-legacy proposals. For legacy ones to pass, the proposer has to be an admin, and the proposal type has to be whitelisted. On the other side, non-legacy proposals are provided as JSON objects, and can be executed and stored only if the admin module is set as a signer, the proposer is an admin, and all the messages are whitelisted. In the end, if everything goes well, both approaches involve creating new proposals, marking them as passed before storing.

In the illustration below we show the workflow of how adminmodule handles an incoming proposal.

2 Workflow of the updated adminmodule During the audit, we inspected the following threats:

Is it possible to submit an invalid JSON string, or to insert some kind of malicious data in order to trick the system: a. in a way that an unauthorized user becomes authority; Resolution: No. There validations for the authority field are in place. b. if there is a possibility of constructing the JSON message with a legacy proposal pattern. Resolution: Also no, because invalid JSON messages simply won't be unmarshalled;
Is there any danger posed by the absence of the nil error check here? Resolution: No, because Cosmos SDK ensures that a handler for the message exists.
Is there a danger of the admin proposal spending too much gas during its execution? Resolution: No, but even if it comes to ‘out of gas’ panic, transaction would be reverted without any further damage.

Fork of the Cosmos SDK slashing module This feature has been backported from Cosmos SDK 0.50 and includes efficiency improvements to the slashing module. It contains a refactored logic for tracking the validator's missed block bitmap, more specifically, the way it is stored and represented. There is a bitmap for each validator which tells which blocks were missed within the current rolling window. The new approach avoids manipulations on entire bitmaps. It actually breaks them into chunks of fixed size of 1024 bits each. When determining whether a specific block was signed or missed by a validator, the system first identifies the chunk where evidence for that particular block is recorded. It then calculates index of the appropriate bit within the identified chunk to retrieve the relevant information. On the other hand, when it comes to setting the new bitmap values, it starts with defining of the appropriate chunk, and setting the bit on the calculated index to true. We paid special attention to functions for setting the bitmap values, those retrieving information from them, as well as those for deleting entire bitmaps. The logic for storing, updating and removing the data is clean and understandable. No issues were found, except that this refactoring is not included in the Cosmos SDK documentation.

Backport of bank-hooks from Osmosis This backport includes the usage of BlockBeforeSend and TrackBeforeSend hooks within the tokenfactory module triggered on DelegateCoins, UndelegateCoins and SendCoin actions inside the bank module. The difference between these two is that for the TrackBeforeSend hook a new context with limited gas amounts needs to be set, since it is used in module-to-module transfers. On the other hand, BlockBeforeSend is avoided when a transfer is happening between two modules.

Within the tokenfactory module, these hooks are implemented in the same function called callBeforeSendListener. This function iterates through each provided coin, sending the corresponding sudo messages to appropriate contract addresses. Depending on the blockBeforeSend boolean flag, it creates and sends either the TrackBeforeSendSudoMsg or BlockBeforeSendSudoMsg.

The primary focus during the audit of this part was on the fact that TrackBeforeSendMsg is designed for module-to-module transfers because it is using additional gas, while it is also used for DelegateCoins and UndelegateCoins functions, which are mostly oriented account-to-module and vice versa. This however does not pose a problem because there is a limit set within the child context.

Findings Title Type Severity Status

Unbounded iteration over Incentives stakes IMPLEMENTATION 4 CRITICAL OUTDATED may lead to chain halt

Stealing of user funds via rounding errors in PROTOCOL 4 CRITICAL RESOLVED pool and limit order swaps IMPLEMENTATION

Stealing of arbitrary funds via IBC swaps IMPLEMENTATION 4 CRITICAL RESOLVED

Quadratic scaling for multi-amount IMPLEMENTATION 3 HIGH RESOLVED deposits may lead to DOS

Contract failures potentially remaining in PROTOCOL 1 LOW RISK ACCEPTED the state indefinitely

Misleading Incentives module user-facing DOCUMENTATION 1 LOW OUTDATED and developer documentation

Limit order tranche security rests on a fine IMPLEMENTATION 1 LOW RISK ACCEPTED interplay of implicit assumptions

Incomplete validation of IMPLEMENTATION 1 LOW RESOLVED MsgMultiHopSwap may lead to user fund loss and frustration

Incomplete validation of pool IMPLEMENTATION 1 LOW RESOLVED denominations

Miscellaneous security concerns IMPLEMENTATION 0X

Excerpt (19992 of 88512 characters). Read the whole page on informalsystems/audits ↗

← Back to Documentation