Missing store revert in case of erc20 conversion error can lead to loss of funds

Lines of code Vulnerability details Impact The module is expected to have no changes in case a erc20 conversion failed. It was implemented by swallowing the error with a log and continuing with the flow finishing the IBC transfer. This is the relevant code section: if , err =...

Potential risk of using swappedAmount in case of swap error

Lines of code Vulnerability details Impact In case the swap operation failed, the module should continue as is with the erc20 conversion and finish the IBC transfer. This is the relevant part of the code that swallows the error: swappedAmount, err = k.coinswapKeeper.TradeInputForExactOutputctx,...

TimeoutTimeStamp and TimeoutHeight fields are not properly validated

Lines of code Vulnerability details Impact The absence of proper validation for TimeoutTimeStamp and TimeoutHeight fields before processing an onboarding request can lead to significant disruption and potential security risks. This might allow an attacker to send an IBC Inter-Blockchain...

Lack of checks for non-zero values

Lines of code Vulnerability details Impact Lack of propper parameter validation Proof of Concept The GetPoolByLptDenom function takes lptDenom as an input parameter and uses it right away as an argument to retrieve a pool from the KVStore. However, if it is empty or invalid, the function will...

packet.DestinationChannel IS CHECKED AGAINST THE WhitelistedChannels, BUT packet.SourceChannel SHOULD BE CHECKED INSTEAD, AS PER THE PROTOCOL DESIGN REQUIREMENTS

Lines of code Vulnerability details Impact In the ibccallbacks.OnRecvPacket function, the Source Channel of the transferred packet is required to be checked against the WhitelistedChannels of the module. If the Source Channel of the packet is not in the WhitelistedChannels list then the auto swap...

Lack of token pair existence

Lines of code Vulnerability details Impact There is no check that ensures the token pair does exists. Proof of Concept he code assumes that if a token pair ID is not registered or if the token pair is not enabled, the conversion process will be skipped. However, there is no explicit check or...

An error could lead to coins not being returned to user

Lines of code Vulnerability details Impact The function swapCoins does check that SendCoins does receive them correctly here but not when sending them to the user here Proof of Concept If there is an error returning the swapped coins to the user, they will remain locked. There is no history of...

Missing store revert in case of swap error can lead to loss of funds

Lines of code Vulnerability details Impact The module is expected to have no state changes in case a swap failed, and continue to the conversion phase. It was implemented by swallowing the error with a log and continuing with the flow erc20 conversion, etc. This is the relevant code section:...

Add check to disallow creation of Standard Denomination pool

Lines of code Vulnerability details Impact CreatePool does not check if the counterpartyDenom is a Standard Denomination. This can lead to creation of pools where the StandardDenom and the CounterpartyDenom are the same. Code reference // CreatePool create a liquidity that saves relevant...

The Swap action will always fail if the value of the deposited IBC asset is less than the value of autoSwapThreshold Canto

Lines of code Vulnerability details Impact The Swap action will always fail if the value of the deposited IBC asset the value of autoSwapThreshold Canto. This is not a bug, but it's very inconvenient for users and makes the goal of the onboarding module fail. Let's asssume that: 1 Canto = 0.1 USD...

GetPoolByLptDenom function does not check if lptDenom string is valid

Lines of code Vulnerability details Impact To check if the lptDenom string is valid The GetPoolByLptDenom function in the keeper package do that job so that is the problem, So an attacker can pass in a malicious lptDenom string that would cause the function to return a non-existent liquidity pool...

GetAllPools could panic at iterator loop

Lines of code Vulnerability details Impact A panic could occur in GetAllPools and stop the program Proof of Concept there is no check for pool value if nil or not before calling MustUnmarshal which could lead to a panic issue in the program func k Keeper GetAllPoolsctx sdk.Context pools types.Poo...

Almost all of the github.com/cosmos/cosmos-sdk/types will be deprecated

Lines of code Vulnerability details Impact Codes in the scope won't work if the would be updated. Proof of Concept It is clearly seen that in the it is fixed version of v0.45.9. However, in next version of cosmos-sdk all of the Int methods will be deprecated. Almost all of the code in the scope i...

The validation of the source channel is performed incorrectly

Lines of code Vulnerability details Impact There is a potential risk of unauthorized sources sending assets to the Canto Network and automatically swapping transferred tokens for Canto tokens. Proof of Concept When bootstrapping Canto Network, node operators config channel ID for the onboarding...

Token pairs that are not whitelisted can be created as a pool

Lines of code Vulnerability details Class Medium Impact In the docs: Only token pairs on the whitelist can be created as a pool. Pool creation fails if the token pair is not on the whitelist. However, there is no logic that prevents from creating non-whitelisted pairs. The check is only happening...

AutoSwapThreshold field is not checked for exceeding MaxAutoSwapThreshold constant

Lines of code Vulnerability details Impact validateAutoSwapThreshold function has a problem that does not check the AutoSwapThreshold field is less than or equal to the MaxAutoSwapThreshold constant, this can caused that a malicious user could set the AutoSwapThreshold field to a value that is...

Missing slippage protection leads to potential sandwich of small transfers or blocking the swap feature

Lines of code Vulnerability details Impact The swap module is invoked with a default of coinswaptypes.InputCoin: transferredCoin, Address: recipient.String, coinswaptypes.OutputCoin: swapCoins, Address: recipient.String. The swap module makes sure that in tokens of the swap are limitted to...

In case when not all RSR was sold on auction users can loose it

Lines of code Vulnerability details Impact In case when not all RSR was sold on auction users can loose it, if era was changed for them. Proof of Concept The purpose of StRSR stakers is to provide RSR tokens, that can back system in case if not enough collateral is present. During rebalance,...

Loss of staking yield for stakers when another user stakes in pause/frozen state

Lines of code Vulnerability details Impact Loss of staking yield for stakers when another user stakes in pause/frozen state. Proof of Concept Issue 148 from previous audit is present again. As i can see it was mitigated. But maybe after that new code changes were made, so this issue is present...

doesn't handle when value of reservePool returned is nil

Lines of code Vulnerability details Impact inputReserve and outputReserve is gotten from the reservePool return However reservePool could be nil and since not handled or checked it results to inaccurate inputReserve and outputReserve Proof of Concept inputReserve and outputReserve are gotten usin...

Incorrect maxSwapAmount checked

Lines of code Vulnerability details Impact On onboarding, a part of the the received tokens will be swapped for 4 canto. To partially protect users, a maximum amount of tokens to be used for the swap is set. According to the documentation: For risk management purposes, a swap will fail if the inp...

Schedule recovery DOS by front-running with original schedule recovery transaction if no other transaction is executed

Lines of code Vulnerability details Description If after scheduling a recovery no transaction is executed, anyone can DOS the execution of this scheduled recovery by a signature replay attack given that the nonce is not increased Impact DOS of scheduled recovery execution if after a recovery is...

M-02 Unmitigated

Lines of code Vulnerability details Mitigation of M-02: Issue partially mitigated, see comments Link to Issue: code-423n4/2023-05-ambire-findings18 Comments While the issue mentioned in M-02 has been technically mitigated, the same attack can be performed in another function present in the wallet...

Insufficient Validation of Addresses Vulnerability

Lines of code Vulnerability details Summary The code suffers from an insufficient validation of addresses vulnerability. Although it uses the sdk.AccAddressFromBech32 function to convert addresses from Bech32 format to sdk.AccAddress, it fails to validate the addresses for their validity and...

M-03 Unmitigated

Lines of code Vulnerability details Mitigation of M-03: Issue not mitigated, see comments Link to Issue: code-423n4/2023-05-ambire-findings16 Comments The issue is not mitigated. There is an error in the implementation of the mitigation of M-03. See report adriro-MR-M-03-ERROR for details. Assess...

potential arithmetic overflow in the code under review.

Lines of code Vulnerability details Summary This report highlights a high vulnerability related to potential arithmetic overflow in the code under review. Vulnerability Details The code performs arithmetic operations, such as addition, subtraction, multiplication, and division, on sdk.Int values...

Inadequate error handling

Lines of code Vulnerability details Summary This report highlights a high vulnerability related to inadequate error handling in the code under review. Vulnerability Details The code lacks proper error handling in certain functions. Specifically, when errors occur, such as in the swapCoins functio...

[adriro-NEW-M-01]: Forced failure of transactions that use tryCatchLimit

Lines of code Vulnerability details adriro-NEW-M-01: Forced failure of transactions that use tryCatchLimit The same attack described in M-02 can also be exploited with transactions that use tryCatchLimit. Impact Similar to tryCatch, the tryCatchLimit function can be used to execute a transaction...

M-03 Unmitigated

Lines of code Vulnerability details adriro-MR-M-03-ERROR: Recovery transaction can be replayed after a cancellation The mitigation of M-03 contains an error in the implementation of the fix. The original issue is still present. Impact The report in M-03 describes an issue related to the replay of...

M-03 Unmitigated

Lines of code Vulnerability details Impact The mitigation updates the following AmbireAccount.execute function by adding nonce++ in the scheduled != 0 && !isCancellation if block within the sigMode == SIGMODERECOVER || sigMode == SIGMODECANCEL if block. However, this does not fix M-03: Recovery...

M-01 Unmitigated

Lines of code Vulnerability details Mitigation of M-01: Issue not mitigated Link to Issue: code-423n4/2023-05-ambire-findings21 Comments Sponsor decided not to mitigate the issue. Assessed type Other --- The text was updated successfully, but these errors were encountered: All reactions...

M-02 Unmitigated

Lines of code Vulnerability details Description The mitigation recommendation is not right. To understand the issue I strongly recommend the lecture of this article. In particular, sections "Insufficient Gas Griefing Attack" and "Workaround Against “Insuficient Gas Griefing attack”". The issue...

tryCatchLimit can forward less than the specified gasLimit due to how CALL* opcode forward gas

Lines of code Vulnerability details Description To understand the issue I strongly recommend the lecture of this article. In particular, sections "Insufficient Gas Griefing Attack" and "Workaround Against “Insuficient Gas Griefing attack”". The problem relays on the fact that we cannot be sure th...

RecollateralizationLibP1.basketRange calculates deficit incorrectly

Lines of code Vulnerability details Impact RecollateralizationLibP1.basketRange calculates deficit incorrectly. so recollateralization may be not efficient. Proof of Concept RecollateralizationLibP1.basketRange calculates range.bottom for pessimistic case. // tok = tok/BU BU uint192 anchor =...

StRSR.cancelUnstake doesn't call _payoutRewards before minting new shares

Lines of code Vulnerability details Impact StRSR.cancelUnstake doesn't call payoutRewards before minting new shares. As result this rewards will be distributed for new staker as well. Proof of Concept If user wants to cancel his withdraw, then he can call StRSR.cancelUnstake and mint new shares...

User who stakes into StRSRVotes doesn't have any voting power

Lines of code Vulnerability details Impact User who stakes into StRSRVotes doesn't have any voting power. This is not intuitive clear and user who thinks that he can vote, actually will not be able until he will delegate votes to himself. Proof of Concept StRSRVotes contract extends StRSR which h...

AssetRegistry.swapRegistered can be called with low gas to make basket be disabled

Lines of code Vulnerability details Impact Attacker can disable basket, when swapping or unregistering not basket asset. Proof of Concept AssetRegistry.swapRegistered and AssetRegistry.unregister are both functions that can be called by governance only. It's possible that after proposal is passed...

StRSR.withdraw can be blocked

Lines of code Vulnerability details Impact StRSR.withdraw can be blocked for user by changing basket's asset to IFFY status. User will not be able to withdraw and his funds can be seized later. Proof of Concept StRSR.withdraw function allows user to withdraw only, when basketHandler.isReady. And...

M-03 Unmitigated

Lines of code Vulnerability details Description The problem identified by both wardens is the chance of using the initial signed recovery transaction to reinitialize the recovery procedure again after its cancellation, as long as this three transaction are done one after other. However the...

[adriro-NEW-M-02]: Wallet design prevents EIP-165 extensibility

Lines of code Vulnerability details adriro-NEW-M-02: Wallet design prevents EIP-165 extensibility The current wallet fallback design prevents the extensibility of the EIP-165 functionality. Impact Ambire wallet extensibility is provided by a fallback mechanism. If a fallback handler is defined in...

DefaultMaxSwapAmount is 10x higher than spec for ETH

Lines of code Vulnerability details Impact In the "coinswap" module a limit is in place for avoiding large swaps and their potential to manipulate the price in a low-liquidity scenario. The spec says: For risk management purposes, a swap will fail if the input coin amount exceeds a pre-defined...

M-10 Unmitigated

Lines of code Vulnerability details Mitigation of M-10: Issue NOT mitigated Mitigated issue M-10: First 1 wei deposit can produce lose of user xETH funds in wxETH Fix: code-423n4/2023-05-xeth@fbb2972 The issue is similar to the standard inflation attack, except that instead of the attacker's...

In case Distributor.setDistribution use, revenue from rToken RevenueTrader and rsr token RevenueTrader should be distributed

Lines of code Vulnerability details Impact In case Distributor.setDistribution use, revenue from rToken RevenueTrader and rsr token RevenueTrader should be distributed. Otherwise wrong distribution will be used. Proof of Concept BackingManager.forwardRevenue function sends revenue amount to the...

M-05 Unmitigated

Lines of code Vulnerability details Mitigation of M-05: Issue NOT mitigated Mitigated issue M-05: Virgin stake can claim all drops Fix: code-423n4/2023-05-xeth@aebc324 The issue is that if dripping is enabled when totalSupply == 0 the entire amount dripped will immediately accrue to the first...

M-07 Unmitigated

Lines of code Vulnerability details Mitigation of M-07: Issue NOT mitigated Mitigated issue M-07: Incorrect slippage check in the AMO2.rebalanceUp can be attacked by MEV Fix: code-423n4/2023-05-xeth@630114e The issue is that since the pool is rebalanced around an imbalanced ratio with 68%-75% xET...

FurnaceP1.setRatio will work incorrect after call when frozen

Lines of code Vulnerability details Impact FurnaceP1.setRatio will not update lastPayout when called in frozen state, which means that after component will be unfrozen, melting will be incorrect. Proof of Concept melt function should burn some amount of tokens from lastPayoutBal. It depends of...

GnosisTrade contract can be frontrunned in order to make it report violation and block broker

Lines of code Vulnerability details Impact GnosisTrade contract can be frontrunned in order to make it report violation and block broker. This will stop broker from creating another traders. Proof of Concept When BackingManager is rebalancing, then it can open trade, which can be gnosis trade...

RToken.redeem should claim rewards before sending tokens to user

Lines of code Vulnerability details Impact RToken.redeem should claim rewards before sending tokens to user. Because after that he will not receive them anymore. Proof of Concept When user mints RToken, then he sends some tokens as collateral and they are stored by BackingManager. Some of...

M-07 Unmitigated

Lines of code Vulnerability details Comments The very first point that needs to be made, is that, according to the Mitigation Review details: In production we have planned to use MEV Protection services such as flashbots rpc The MEV Protection rpc ensure the rebalance and defender won't be affect...

Upgraded Q -> 2 from #66 [1686923855595]

Judge has assessed an item in Issue 66 as 2 risk. The relevant finding follows: L-13 The owner is a single point of failure and a centralization risk Having a single EOA as the only owner of contracts is a large centralization risk and a single point of failure. A single private key may be taken ...