Centrifuge - Sathish9098's results

LOW FINDINGS

[L-1] The owner is a single point of failure and a centralization risk

Impact

Having a single EOA as the owner of contracts is a large centralization risk and a single point of failure. A single private key may be taken in a hack, or the sole holder of the key may become unable to retrieve the key when necessary.

POC

FILE: 2023-09-centrifuge/src/LiquidityPool.sol

214: function requestDeposit(uint256 assets, address owner) public withApproval(owner) {

231: function requestRedeem(uint256 shares, address owner) public withApproval(owner) {

247: function decreaseDepositRequest(uint256 assets, address owner) public withApproval(owner) {

253: function decreaseRedeemRequest(uint256 shares, address owner) public withApproval(owner) {

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/LiquidityPool.sol#L214

Recommended Mitigation

Consider changing to a multi-signature setup, or having a role-based authorization model.

[L-2] approve()/safeApprove() may revert if the current approval is not zero

Impact

Calling approve() without first calling approve(0) if the current approval is non-zero will revert with some tokens, such as Tether (USDT). While Tether is known to do this, it applies to other tokens as well, which are trying to protect against this attack vector. safeApprove() itself also implements this protection. Always reset the approval to zero before changing it to a new value, or use safeIncreaseAllowance()/safeDecreaseAllowance()

POC

FILE: 2023-09-centrifuge/src/token/ERC20.sol

131: function approve(address spender, uint256 value) external returns (bool) {

FILE: 2023-09-centrifuge/src/PoolManager.sol

249: EscrowLike(escrow).approve(currencyAddress, investmentManager.userEscrow(), type(uint256).max);

252: EscrowLike(escrow).approve(currencyAddress, address(investmentManager), type(uint256).max);

261: EscrowLike(escrow).approve(currencyAddress, address(this), amount);

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/PoolManager.sol#L249

Recommended Mitigation

First approve(0) then approve the amount

[L-3] approve() return value not checked

The approve() function returns a boolean value indicating whether or not the approval was successful. If the approval was successful, the return value will be true. If the approval was not successful, the return value will be false.

Not checking the return value of the approve() function can lead to security vulnerabilities.

FILE: 2023-09-centrifuge/src/PoolManager.sol

249: EscrowLike(escrow).approve(currencyAddress, investmentManager.userEscrow(), type(uint256).max);

252: EscrowLike(escrow).approve(currencyAddress, address(investmentManager), type(uint256).max);

261: EscrowLike(escrow).approve(currencyAddress, address(this), amount);

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/PoolManager.sol#L249

Recommended Mitigation

Check the status of the approve() function

[L-4] External call recipient may consume all transaction gas

Impact

There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert. Use addr.call{gas: <amount>}("") or this library instead.

POC

FILE: Breadcrumbs2023-09-centrifuge/src/LiquidityPool.sol

296: (bool success, bytes memory data) = address(share).call(bytes.concat(msg.data, bytes20(msg.sender)));

302: (bool success, bytes memory data) = address(share).call(bytes.concat(msg.data, bytes20(msg.sender)));

308: (bool success, bytes memory data) = address(share).call(bytes.concat(msg.data, bytes20(msg.sender)));

314: (bool success,) = address(share).call(bytes.concat(msg.data, bytes20(address(this))));

319: (bool success,) = address(share).call(bytes.concat(msg.data, bytes20(address(this))));

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/LiquidityPool.sol#L296

[L-5] Solidity version 0.8.21 may not work on other chains due to PUSH0

Impact

The compiler for Solidity 0.8.21 switches the default target EVM version to Shanghai, which includes the new PUSH0 op code. This op code may not yet be implemented on all L2s, so deployment on these chains will fail. To work around this issue, use an earlier EVM version. While the project itself may or may not compile with 0.8.21, other projects with which it integrates, or which extend this project may, and those projects will have problems deploying these contracts/libraries.

All contracts and librarires using the solidity version 0.8.21. Still PUSH0 opcode is not deprecated

POC

FILE: Breadcrumbs2023-09-centrifuge/src/LiquidityPool.sol

2: pragma solidity 0.8.21;

Recommended Mitgation

Use lower version solidity like 0.8.19

[L-6] Unsafe downcast

Impact

When a type is downcast to a smaller type, the higher order bits are truncated, effectively applying a modulo to the original value. Without any other checks, this wrapping will lead to unexpected behavior and bugs

_value uint256 is unsafely down casted to uint128

POC

FILE: 2023-09-centrifuge/src/InvestmentManager.sol

670: value = uint128(_value);

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L670

Recommended Mitgation

Use OpenZeppelin SafeCast Libraries

[L-7] Using block.timestamp as the deadline/expiry invites MEV

Impact

Passing block.timestamp as the expiry/deadline of an operation does not mean require immediate execution - it means whatever block this transaction appears in, I'm comfortable with that block's timestamp. Providing this value means that a malicious miner can hold the transaction for as long as they like (think the flashbots mempool for bundling transactions), which may be until they are able to cause the transaction to incur the maximum amount of slippage allowed by the slippage parameter, or until conditions become unfavorable enough that other orders, e.g. liquidations, are triggered. Timestamps should be chosen off-chain, and should be specified by the caller to avoid unnecessary MEV.

POC

FILE: 2023-09-centrifuge/src/token/RestrictionManager.sol

46: require((members[user] >= block.timestamp), "RestrictionManager/destination-not-a-member");
50: if (members[user] >= block.timestamp) {
58: require(block.timestamp <= validUntil, "RestrictionManager/invalid-valid-until");

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/RestrictionManager.sol#L50

FILE: 2023-09-centrifuge/src/Root.sol

77: require(schedule[target] < block.timestamp, "Root/target-not-ready");

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/Root.sol#L77

FILE: 2023-09-centrifuge/src/token/ERC20.sol

217: require(block.timestamp <= deadline, "ERC20/permit-expired");

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L217

Recommended Mitgation

Timestamps should be chosen off-chain, and should be specified by the caller

[L-8] MAX_DELAY should be configurable to avoid problems in future

If there are any future changes, it can be difficult to modify the contract, and redeployment is currently the only viable option. Therefore, it is advisable to add a setter function for MAX_DELAY to facilitate future adjustments.

FILE: 2023-09-centrifuge/src/Root.sol

17: uint256 private MAX_DELAY = 4 weeks;

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/Root.sol#L17

Recommended Mitigation

Add setter function

function setMaxDelay(uint256 _newMaxDelay) external onlyOwner {
        MAX_DELAY = _newMaxDelay;
    }

[L-9] Shadowed variable in parameters should be avoided

Shadowed variables in parameters should be avoided to maintain code clarity, avoid unintended side effects, and make debugging easier. When you shadow a variable in a function's parameters, it can lead to confusion and unexpected behavior.

In Factory.sol and Auth.sol contracts same wards variable

Factory.sol used wards as function parameter and Auth.sol used wards as state variable

FILE: 2023-09-centrifuge/src/util/Factory.sol

42: address[] calldata wards

47: for (uint256 i = 0; i < wards.length; i++) {
48: liquidityPool.rely(wards[i]);

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/util/Factory.sol#L42

Recommended Mitigation

- 42: address[] calldata wards
+ 42: address[] calldata _wards

[L-10] Assigning values to state variables without performing sanity checks on integer values

Integer values are being assigned directly to state variables or immutables without any validation or verification to ensure that the assigned values are within acceptable or "sane" ranges . It's crucial to validate data before updating state variables to prevent unexpected behavior or vulnerabilities.

FILE: 2023-09-centrifuge/src/Root.sol

36: delay = _delay;

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/Root.sol#L36

If decimals immutable variable sets wrong value then this will cause unintended consequences. Can't changed once value set

FILE: 2023-09-centrifuge/src/token/ERC20.sol

43:    decimals = decimals_;

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L43

Recommended Mitigation

Add sanity checks for integer like > 0

require(_delay > 0, " wrong value");

[L-11] Using zero as a parameter

Taking zero as a valid argument without checks can lead to severe security issues in some cases.

If using a zero argument is mandatory, consider using descriptive constants or an enum instead of passing zero directly on function calls, as that might be error-prone, to fully describe the caller's intention.

FILE: 2023-09-centrifuge/src/InvestmentManager.sol

177: require(liquidityPool.checkTransferRestriction(address(0), user, 0), "InvestmentManager/not-a-member");

190: require(liquidityPool.checkTransferRestriction(address(0), user, 0), "InvestmentManager/not-a-member");

202: require(liquidityPool.checkTransferRestriction(address(0), user, 0), "InvestmentManager/not-a-member");

213: require(liquidityPool.checkTransferRestriction(address(0), user, 0), "InvestmentManager/not-a-member");

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L177

[L-12] no checks for source and destination addresses being the same

The function transferIn() in the UserEscrow contract does not check if the source and destination addresses are the same. This means that a user could potentially deposit tokens into their own escrow address.if an attacker is able to compromise a user's escrow account, they could potentially steal all of the tokens in the account, even if the tokens are deposited into the user's own escrow address.

FILE: 2023-09-centrifuge/src/UserEscrow.sol

// --- Token transfers ---
    function transferIn(address token, address source, address destination, uint256 amount) external auth {
        destinations[token][destination] += amount;

        SafeTransferLib.safeTransferFrom(token, source, address(this), amount);
        emit TransferIn(token, source, destination, amount);
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/UserEscrow.sol#L29

Recommended Mitigation

require(source != destination, "wrong address");

[L-13] transferIn function lacks the token support checks in UserEscrow contract

Impact

The function does not check if the token is supported by the escrow. This means that users could potentially deposit tokens into the escrow that cannot be withdrawn.

FILE: 2023-09-centrifuge/src/UserEscrow.sol

// --- Token transfers ---
    function transferIn(address token, address source, address destination, uint256 amount) external auth {
        destinations[token][destination] += amount;

        SafeTransferLib.safeTransferFrom(token, source, address(this), amount);
        emit TransferIn(token, source, destination, amount);
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/UserEscrow.sol#L29

Recommended Mitgation

Add a check to ensure that the token is supported by the escrow

[L-14] transferOut() function no check for receiver being a contract

The function does not check if the receiver is a contract. This means that an attacker could potentially use a malicious contract to steal the tokens from the destination

FILE: 2023-09-centrifuge/src/UserEscrow.sol

function transferOut(address token, address destination, address receiver, uint256 amount) external auth {
        require(destinations[token][destination] >= amount, "UserEscrow/transfer-failed");
        require(
            /// @dev transferOut can only be initiated by the destination address or an authorized admin.
            ///      The check is just an additional protection to secure destination funds in case of compromized auth.
            ///      Since userEscrow is not able to decrease the allowance for the receiver,
            ///      a transfer is only possible in case receiver has received the full allowance from destination address.
            receiver == destination || (ERC20Like(token).allowance(destination, receiver) == type(uint256).max),
            "UserEscrow/receiver-has-no-allowance"
        );
        destinations[token][destination] -= amount;

        SafeTransferLib.safeTransfer(token, receiver, amount);
        emit TransferOut(token, receiver, amount);
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/UserEscrow.sol#L36-L50

Recommended Mitigation

Add a check to ensure that the receiver is not a contract

[L-15] Add a timelock to the transferOut() function to prevent attackers from withdrawing funds immediately after compromising a user's escrow account

Impact

To add a timelock to the transferOut() function in the UserEscrow contract, you can add a new state variable called timelock and a new modifier called withTimelock. The timelock variable will store the timestamp at which a transfer was requested, and the withTimelock modifier will ensure that a transfer can only be executed after the timelock period has elapsed.

The timelock period can be set to any desired value, such as 24 hours or 7 days. This will prevent attackers from withdrawing funds immediately after compromising a user's escrow account.

FILE: 2023-09-centrifuge/src/UserEscrow.sol

function transferOut(address token, address destination, address receiver, uint256 amount) external auth {
        require(destinations[token][destination] >= amount, "UserEscrow/transfer-failed");
        require(
            /// @dev transferOut can only be initiated by the destination address or an authorized admin.
            ///      The check is just an additional protection to secure destination funds in case of compromized auth.
            ///      Since userEscrow is not able to decrease the allowance for the receiver,
            ///      a transfer is only possible in case receiver has received the full allowance from destination address.
            receiver == destination || (ERC20Like(token).allowance(destination, receiver) == type(uint256).max),
            "UserEscrow/receiver-has-no-allowance"
        );
        destinations[token][destination] -= amount;

        SafeTransferLib.safeTransfer(token, receiver, amount);
        emit TransferOut(token, receiver, amount);
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/UserEscrow.sol#L36-L50

Recommended Mitigation

Add timelock

modifier withTimelock() {
    require(block.timestamp >= timelock, "UserEscrow/timelock-not-expired");
    _;
}

Analysis - Centrifuge

Summary

1. Overview

The institutional ecosystem for on-chain credit

Centrifuge is the institutional platform for credit onchain. Centrifuge was the first protocol where MakerDAO minted DAI against a real-world asset, the first onchain securitization, and Centrifuge launched the RWA Market with Aave.

Centrifuge works based on a hub-and-spoke model.Liquidity Pools are deployed on any other L1 or L2 where there is demand for RWA, and each Liquidity Pool deployment communicates directly with Centrifuge Chain using messaging layers

2. Approach taken in evaluating the codebase

• Preliminary analysis: I read the contest Readme.md file and took the following notes:

• The Centrifuge learnings,

  • Composition
  • Test coverage is only 80%
  • Only ERC20 transfers
  • Timelock, DeFi, Multi-chain

• Areas to focus

  • Loss of funds
  • Stuck funds
  • Bypass of timelock
  • Price manipulationns

• High-level overview : I analyzed the overall codebase in one iteration to get a high-level understanding of the code structure and functionality.

• Documentation review : I studied the documentation to understand the purpose of each contract, its functionality, and how it is connected with other contracts.

• Literature review : I read old audits and known findings, as well as the bot races findings.

• Testing setup : I set up my testing environment and ran the tests to ensure that all tests passed. I used Foundry to test the Centrifuge , and I used forge comments for testing.

  • forge build
  • forge test

• Detailed analysis : I began by conducting a detailed analysis of the codebase, going through it line by line. I meticulously took notes to prepare questions for the sponsors. I used @audit tags to identify and flag vulnerable and weak parts in the codebase. Subsequently, I delved into a thorough analysis and conducted all the necessary unit and fuzz tests to ensure the protocol functions as intended

2.1 Learnings

Centrifuge serves as the institutional platform for on-chain credit. Notably, Centrifuge pioneered several key advancements in DeFi:

• Real-World Asset Backing : Centrifuge was the first protocol where MakerDAO minted DAI against real-world assets, bridging traditional finance with the blockchain world.
• On-chain Securitization: It also achieved the first on-chain securitization, enabling the tokenization of real-world assets for investment.
• RWA Market with Aave : Centrifuge played a pivotal role in launching the RWA Market with Aave, further expanding the use cases for real-world assets in DeFi.

Core contract components

• Liquidity Pool: An ERC-4626 compatible contract allowing investors to deposit and withdraw stablecoins for investing in tranches of pools.
• Tranche Token : An ERC-20 token representing different tranches, each linked to a RestrictionManager managing transfer restrictions. Tranche token prices are computed on Centrifuge.
• Investment Manager: The primary contract for pool creation, tranche deployment, investment management, and token handling.
• Pool Manager : A secondary contract for currency bookkeeping and handling tranche token transfers and currencies.
• Gateway : An intermediary contract responsible for encoding and decoding messages and routing them to/from Centrifuge.
• Routers: Contracts handling message communication to/from Centrifuge Chain.

3. Architecture recommendations

Here are some architecture related suggestions that could be made to improve the Centrifuge.

After conducting a comprehensive analysis of the protocol, I have derived the following technical architecture suggestions aimed at enhancing the efficiency and effectiveness of the protocol

Implement a more efficient communication mechanism between Liquidity Pools and Centrifuge Chain

The current communication mechanism between Liquidity Pools and Centrifuge Chain uses external general message passing protocols. This means that Liquidity Pools need to send and receive messages to and from Centrifuge Chain through a Routers. This can be inefficient and slow for a number of reasons:

• Message overhead : Each message that is sent between Liquidity Pools and Centrifuge Chain needs to be encoded and decoded, which adds overhead.
• Latency : There is a delay between the time that a message is sent from Liquidity Pools to Centrifuge Chain and the time that the message is received.

Routers are responsible for routing messages between Liquidity Pools and Centrifuge Chain.The current communication mechanism between Liquidity Pools and Centrifuge Chain uses Routers to send and receive messages.Routers are an important part of the Centrifuge protocol, but they could potentially be replaced by a more efficient and secure communication mechanism in the future.

• One possible way to implement a more efficient and secure communication mechanism between Liquidity Pools and Centrifuge Chain instead of Routers is to use a dedicated messaging protocol .
• Another possible way to implement a more efficient and secure communication mechanism is to integrate Liquidity Pools directly into the Centrifuge Chain codebase. This would allow Liquidity Pools to communicate with Centrifuge Chain directly, without the need for Routers.

Improve the support for multiple currencies

The current support for multiple currencies in Liquidity Pools is limited. Liquidity Pools can only be linked to a single tranche token, and there is no support for currencies with more than 18 decimals.

• One possible improvement would be to allow Liquidity Pools to be linked to multiple tranche tokens. This would give investors more flexibility in how they choose to invest in real-world assets.

• Another possible improvement would be to support currencies with more than 18 decimals. This would make Centrifuge more accessible to a wider range of investors.

Use a more efficient data structure for representing tranches

The current data structure for representing tranches is a bit inefficient. It could be improved by using a more efficient data structure, such as a Merkle tree.

Using a Merkle tree to represent tranches could be a more efficient way to store and manage tranche data. A Merkle tree is a data structure that allows for efficient verification of the integrity of data.

4. Codebase quality analysis

1. While you have some comments in your contract, there are places where more detailed explanations would be beneficial. Consider adding comments to explain complex logic, especially in functions like convertToShares and convertToAssets
2. You have some magic numbers in your code, such as 10 ** (PRICE_DECIMALS + trancheTokenDecimals - currencyDecimals). Consider replacing these with named constants or variables with meaningful names to improve code readability and maintainability
3. It's good that you're providing error messages in your require statements. However, consider adding more descriptive error messages that provide specific information about the error condition.
4. Ensure consistency in naming conventions. For example, you have both _trancheTokenAmount and trancheTokenAmount, which can be confusing. Stick to one naming convention throughout your contract
5. Ensure that all user inputs are properly validated. For example, in functions like requestDeposit and requestRedeem, it's important to validate user inputs and handle edge cases gracefully.
6. Be mindful of gas costs, especially in functions like requestDeposit and requestRedeem.
7. Evaluate whether any contract parameters should be set as immutable after deployment to enhance security and prevent unintended changes.
8. While the contract uses the MathLib library for some mathematical operations, it's a good practice to use safe math libraries like OpenZeppelin's SafeMath to prevent integer overflow and underflow vulnerabilities.
9. Consider breaking down the contract into smaller, more focused contracts. This can improve readability and make it easier to understand and maintain
10. Instead of repeatedly converting between uint256 and uint128 using _toUint128 and vice versa, consider doing these conversions once where needed and storing the results in variables. This reduces code clutter
11. Consider adding reentrancy protection using the nonReentrant modifier or similar techniques if there are any external contract calls.
12. Consider implementing mechanisms to mitigate front-running attacks if applicable, especially in functions related to price updates.
13. It can be further improved by providing more detailed explanations for complex functions or logic with more comments

5. Centralization risks

The use of the auth and withApproval(owner) mechanisms plays a significant role in enabling many contracts to execute critical functions.

function withdraw(uint256 assets, address receiver, address owner)
        public
        withApproval(owner)
        
function mint(address, uint256) public auth {

function burn(address, uint256) public auth {

function updatePrice(uint128 price) public auth {

Centralized control can create single points of failure. If the addresses with special privileges are compromised, mishandled, or become unresponsive, it can disrupt or jeopardize the protocol's operations. This centralization of power is especially risky in decentralized and trust-minimized systems.

Mitigations to avoid centralization:

• Rotate admin roles regularly: The admin roles should be rotated regularly to mitigate the risk of compromise. This can be done by transferring the roles to different addresses or by using a timelock mechanism.
• Use multi-signature wallets: Multi-signature wallets require multiple signatures to approve a transaction. This can help to prevent unauthorized changes to the contract.
• Instead of relying on a single administrative role with full control, consider splitting administrative privileges into specific roles.
• To enhance the robustness of your contract and handle unexpected behaviors or emergencies, you can consider adding setter functions and an emergency pause mechanism.

6.Mechanism review

The Centrifuge protocol is a well-designed and implemented system for bringing real-world assets (RWAs) on-chain. The protocol uses a hub-and-spoke model, with a central Centrifuge Chain and Liquidity Pools deployed on other L1s and L2s. This allows investors to access RWA yields on the network of their choice.

The mechanism's used to improve Centrifuge Protocol

The ward pattern for authentication

This pattern allows for fine-grained control over who has access to which contracts and functions

Contract migrations instead of upgradeable proxies

This makes the protocol more secure and resistant to hacks

Few dependencies

This makes the protocol more reliable and less likely to be affected by bugs in other projects.

Asynchronous deposits and redemptions through an epoch mechanism

This reduces the risk of front-running and other attacks.

A compacted ABI encoding scheme for messages between Liquidity Pools and Centrifuge Chain

This improves the efficiency of communication between the two chains

7. Systemic Risks

Based on an analysis of the protocol's documentation and codebase implementations, several potential systemic risks have been identified. These risks, rooted in technical and operational aspects, have the potential to impact the protocol's stability and reliability.

Router attacks

Routers are a critical part of the Centrifuge protocol, as they are responsible for sending and receiving messages between Liquidity Pools and Centrifuge Chain. If an attacker were to gain control of a router, they could potentially steal funds or disrupt the protocol.

Escrow risk

The Escrow contract holds all of the funds that are deposited by investors. If the Escrow contract were to be hacked, the attackers could steal all of the funds.

Liquidity risk

Liquidity Pools need to have sufficient liquidity in order to meet all of the redemption requests from investors. If there is not enough liquidity in a Liquidity Pool, investors may not be able to redeem their tokens.

Emergency Handling Complexity

The protocol's emergency handling procedures involve multiple steps, time delays, and interactions between administrators. While these procedures are designed to enhance security, their complexity may introduce operational risks if not executed accurately.

Price Volatility

Price oracles play a critical role in determining asset values within the protocol. Price volatility or manipulation in the oracle data sources can lead to inaccurate asset pricing, affecting user balances and transactions.

Cross-Chain Risks

If the protocol interacts with multiple blockchain networks, cross-chain interoperability risks emerge. Issues related to different blockchain protocols, delays, or communication failures may impact the movement of assets between chains.

Non-Standard Token Support

Supporting tokens with varying decimals and unconventional properties requires meticulous handling to prevent calculation errors or discrepancies in asset values.

7.1. Systemic risk as per codebase analysis

• The updatePrice() function lacks validation when assigning values to the latestPrice variable. Although only authorized users can update the price, it's essential to incorporate validation to prevent potential human errors from impacting the protocol flow. Additionally, it's crucial to establish limits when assigning a price to prevent any extreme values that could disrupt the protocol.
• LiquidityPool contract does not appear to implement specific safeguards to protect liquidity pools against liquidity shortages, such as withdrawal limits, cooldown periods, or reserve funds. While the code includes functions for depositing, withdrawing, and managing shares, it does not contain explicit logic or mechanisms for managing liquidity risk. The code focuses more on the core functionality of handling deposits, shares, and price updates, but it lacks the safeguards commonly implemented in DeFi protocols to mitigate liquidity risks.Safeguards to prioritize when protecting liquidity pools consider Withdrawal Limits , Cooldown Periods, Dynamic Fees, Reserve Fund , Risk Assessment , Emergency Shutdown.
• Relying on the rely and deny functions for ownership and control assumes that these functions are secure.
• Consider implementing a fallback function (fallback or receive) to handle unexpected or accidental Ether transfers to the contract. This function should revert to prevent Ether from being trapped
• Develop and document clear emergency shutdown procedures to protect user funds and the protocol in the event of critical vulnerabilities, attacks, or other unforeseen circumstances.
• SafeTransferLib.sol is adapted the TransferHelper.sol and this contract uses a number of expensive operations, such as transferFrom() and approve(). These operations can increase the gas cost of transactions, especially when they are used multiple times in a single transaction
• The TransferHelper.sol contract is not designed to be used with all types of tokens. For example, the contract cannot be used to transfer tokens that have a different ERC20 implementation than the standard ERC20 implementation
• The auth modifier is used for access control. However, it's unclear from the provided code who has the auth privilege
• The contract allows anyone with the appropriate permissions to transfer out tokens from the escrow without specifying withdrawal limits. Depending on the use case, this lack of withdrawal limits could be a security risk, especially if unauthorized parties gain access
• There's no provision for revoking access or permissions once granted. If access should be time-limited or revoked for any reason, this should be addressed in the contract
• The code uses type(uint256).max as a comparison value. While this can be valid in some cases, it's essential to ensure that using the maximum value doesn't lead to unintended consequences or vulnerabilities
• While events are used for transparency, they should not reveal sensitive data. Depending on the contract's purpose, revealing the exact amounts and addresses in events could pose privacy risks.

8. Time spent on analysis

15 Hours

Time spent:

15 hours