Liquidity Management

Summary

This report details several vulnerabilities found in the Gamma Strategies smart contracts, ranging from high to low severity. The high severity vulnerability relates to a lack of checks on deposit amounts, potentially leading to incorrect staking. Medium severity issues include price manipulation risks, privileged function vulnerabilities, and Chainlink price feed validation problems. Low severity findings cover potential division by zero, gas griefing, missing address checks, and deviations from best coding practices.

[High-1] Lack of Validation on Arbitrary Deposit Amounts Could Lead to Incorrect Staking for Certain Tokens (e.g., cUSDCv3)

Description: The deposit function in PerpetualVault.sol does not validate whether the amount parameter is equal to type(uint256).max. Certain tokens, such as cUSDCv3, have special transfer logic where, if the transfer amount is set to type(uint256).max, the contract automatically transfers the sender's entire balance instead of the maximum possible uint256 value.

Vulnerability Details

• If a user with a small balance of cUSDCv3 attempts to deposit type(uint256).max, the function records the deposit as type(uint256).max, but in reality, only the sender's total balance is transferred.

• This leads to incorrect accounting within the protocol, as the system believes it has received a much larger amount than what was actually transferred.

• Such discrepancies can result in unexpected behaviors, including incorrect staking calculations, reward distributions, or contract failures.

Impact

This vulnerability creates a critical inconsistency in the protocol’s accounting system:

• Incorrect Deposit Tracking: If a user with a small balance of cUSDCv3 attempts to deposit type(uint256).max, the system incorrectly registers the deposit as type(uint256).max, while only the user's total balance is transferred.

• Miscalculated Rewards & Staking Allocations: Since the deposit amount is overestimated, rewards, interest, or staking benefits might be misallocated, leading to potential unfair advantages or losses for other users.

• Potential Exploitation & Contract Failures: Malicious actors could manipulate this flaw to disrupt the protocol’s expected behavior, triggering unintended logic that could cause financial losses or smart contract failures.

Recommendations

To prevent this issue, the following safeguards should be implemented:

1. Enforce Input Validation:

• Explicitly reject type(uint256).max as a valid deposit amount to prevent unintended behavior.

• Example:

1. Use Safe Transfer Logic:

• Ensure the actual amount transferred matches the expected deposit amount before updating protocol state.

• Example:

1. Implement Token-Specific Handling:

• For tokens with custom transfer logic (e.g., cUSDCv3), introduce a whitelist or special handling mechanism to properly validate deposit amounts.
  Conduct Extensive Testing:

1. Use fuzzing tests and simulations to check deposit behavior with various token types and edge cases.

• By implementing these fixes, the protocol can ensure deposit amounts are accurately recorded, preventing unintended staking issues and potential exploits. 🚀

[Medium-1] Price is calculated onchain using balanceOf value which is prone to manipulation

Description:

Calculating price on-chain using balanceOf values is inherently prone to manipulation, especially in scenarios where an attacker can directly influence the token balances being used in the calculation. For instance, by transferring large amounts of tokens to or from the relevant contract, an attacker can skew the balance and, consequently, the calculated price. This is particularly problematic in DeFi systems like AMMs, staking contracts, or reward distribution mechanisms, where accurate price computation is critical.

To mitigate this risk, price calculations should use more robust methods, such as time-weighted average prices (TWAPs) or trusted oracle services. These approaches provide resistance to manipulation by considering historical data or off-chain sources, ensuring calculations reflect accurate and tamper-resistant values. Avoid relying solely on token balances, as they are highly susceptible to intentional exploitation such as donation attacks, potentially compromising the integrity and security of the protocol.

Num of instances: 2

Vulnerability Details

Impact

The impact of relying on balanceOf values for price calculation can lead to severe vulnerabilities in the system. Since token balances can be manipulated by an attacker through transactions that modify the balance of tokens held by the contract, the calculated price could be easily skewed. This manipulation could affect the accuracy of price feeds in decentralized finance (DeFi) applications, such as automated market makers (AMMs), staking contracts, or reward systems.

If the price is calculated inaccurately, it could cause improper reward distributions, mispricing of assets, or allow attackers to profit from exploiting the vulnerability. This could result in the loss of funds, compromised security of the protocol, and potentially loss of user trust.

By using balanceOf, which is susceptible to direct manipulation, the integrity of the price calculation and the overall protocol is at risk, particularly in high-volume environments where attackers may have sufficient resources to influence balances and cause significant financial harm.

Recommendations

To address the vulnerabilities associated with using balanceOf values for price calculation, the following recommendations should be considered:

1. Use Time-Weighted Average Prices (TWAPs): Implementing TWAPs can help reduce the impact of short-term fluctuations in token balances. By averaging the price over a defined period, you can minimize the influence of transient manipulation attempts, leading to a more stable price calculation.

2. Integrate Trusted Oracle Services: Utilize trusted external oracles, such as Chainlink or Band Protocol, to provide tamper-resistant price feeds. Oracles aggregate off-chain data and supply it to the smart contract, providing a more reliable and secure means of calculating prices.

3. Avoid Solely Relying on Token Balances: Where possible, avoid using token balances directly in price calculations. Instead, consider leveraging a combination of off-chain data, aggregate metrics, and robust price feeds that are less susceptible to manipulation.

4. Implement Safeguards Against Flash Loan Attacks: Flash loans could be used to manipulate token balances for a short duration, skewing the price calculation. Design your price calculation mechanism to consider the possibility of flash loan manipulation and implement safeguards to minimize their impact.

5. Monitor and Update Price Feeds Regularly: Periodically review and update the price calculation mechanisms to ensure they remain secure against evolving attack strategies. This could include adjusting the weightings of price sources or implementing more advanced techniques as the ecosystem evolves.

[Medium-2] Privileged functions can create points of failure

Summary

The contract contains multiple privileged functions that are restricted to the contract owner using the onlyOwner modifier. While this pattern is common in many contracts, it introduces the risk of a single point of failure (SPOF) by centralizing control within one account. If the private key of the owner account is compromised, the entire contract's functionality can be manipulated, leading to potential security risks. These functions may include settings related to the vault state, treasury, keeper, deposit status, and others.

Vulnerability Details

Privileged functions like setVaultState, setKeeper, setTreasury, setMinMaxDepositAmount, and others in the PerpetualVault.sol and GmxProxy.sol contracts are restricted to the owner. The use of the onlyOwner modifier ensures that only the contract owner can invoke these functions, but it also introduces the possibility of compromise if the owner’s private key is lost or stolen. This design can create a bottleneck in operations, making the system highly vulnerable to malicious attacks targeting the owner’s account.

In total, 17 instances of such privileged functions are present, which could collectively result in severe disruptions or malicious activities if any of the owner accounts are compromised.

Impact

Single Point of Failure (SPOF): Centralizing control in one account introduces a critical failure point. If the private key of the owner account is compromised, an attacker could manipulate the contract and perform unauthorized actions.
Loss of Funds: Given the nature of the functions (e.g., treasury management, vault state changes), malicious actions could lead to significant financial losses.
Increased Risk in Multi-Contract Systems: In a multi-contract environment (like this one, with interactions across PerpetualVault.sol, GmxProxy.sol, and KeeperProxy.sol), the compromised owner account can lead to cascading failures across the system, affecting the entire ecosystem.

Recommendations

Implement Multi-Signature Wallets (Multi-Sig): Instead of relying on a single owner, utilize a multi-signature wallet to require multiple signatures from different parties for critical actions. This ensures that no single individual can control the contract, mitigating the risk of compromise.
Decentralized Control: Consider distributing administrative control across several trusted parties. Implementing decentralized governance mechanisms, such as a DAO (Decentralized Autonomous Organization), could be a viable approach to increase the security and resilience of the system.
Use Time-Locks: Implement a time-lock for privileged functions to introduce a delay between function calls and their execution. This delay provides a window of opportunity for stakeholders to react in case of suspicious activities.

[Medium-3] Contract doesn't initialize inherited OZ Upgradeable contracts

Summary

The contract inherits from OpenZeppelin’s (OZ) upgradeable contracts (such as OwnableUpgradeable and ReentrancyGuardUpgradeable), but fails to initialize them properly using the __Ownable_init() method or equivalent initialization functions. This oversight could lead to improper setup of state variables and internal logic, potentially compromising the contract's upgradeable functionality.

Vulnerability Details

When inheriting from upgradeable contracts like OwnableUpgradeable and ReentrancyGuardUpgradeable, it's crucial to call their respective initializer functions during contract deployment to properly set up their internal state. Failure to do so means that the inherited functionality (such as ownership checks or reentrancy protections) may not be set correctly, leaving the contract vulnerable to security breaches and functional issues.

In this case, the KeeperProxy.sol contract includes the Ownable2StepUpgradeable and ReentrancyGuardUpgradeable contracts, but there is no initialization logic to invoke the necessary setup methods (e.g., __Ownable_init() for OwnableUpgradeable). As a result, the contract may fail to work as expected, and attackers could exploit this flaw to bypass ownership restrictions or perform reentrancy attacks.

['25']

Impact

Improper Access Control: Without calling the initialization functions for the OwnableUpgradeable contract, the contract's owner cannot be properly set, which could allow unauthorized users to control the contract.
Reentrancy Vulnerabilities: If the ReentrancyGuardUpgradeable contract is not initialized, the contract might be susceptible to reentrancy attacks, where an attacker could exploit recursive calls to manipulate contract state or steal funds.
Potential for Exploitation: An attacker could exploit the lack of initialization to bypass critical security measures, resulting in potential contract misbehaviors, such as unauthorized access to funds or malicious operations.

Recommendations

Proper Initialization of Upgradeable Contracts: Ensure that all inherited upgradeable contracts are initialized properly by calling their respective initialization functions (e.g., **Ownable_init() for OwnableUpgradeable, **ReentrancyGuard_init() for ReentrancyGuardUpgradeable).
Use of OpenZeppelin's Upgradeable Contracts Best Practices: Follow OpenZeppelin’s best practices for upgradeable contracts, which include always calling the initializer functions in the constructor or initialize() function, as per the guidelines provided in the official documentation.
Thorough Testing of Initialization Logic: Test the initialization sequence thoroughly to ensure that the contract's state variables and functionality are set up correctly upon deployment.

[Medium-4] Chainlink price feeds are not validated

Summary

The contract utilizes Chainlink’s latestRoundData function to fetch price information from an oracle feed. However, the prices returned by the oracle are not validated before being used in calculations. If the price is zero, negative, or out of bounds, it can result in incorrect contract behavior, particularly when interacting with unsigned integers (uint). This can lead to underflows, where the value wraps around and causes unexpected large numbers that can break the contract’s logic. To prevent this, the contract should validate that the price data returned by the oracle is within expected bounds, positive, non-zero, and safe for use in arithmetic operations.

Vulnerability Details

In the current implementation, the contract fetches price data from the Chainlink oracle using latestRoundData. However, there is no validation of the values returned by the oracle. Specifically:

1. Zero or negative prices: The code does not check whether the price returned by Chainlink’s oracle is zero or negative. Since uint types cannot handle negative numbers, any negative or zero price would result in invalid contract behavior.

2. Price offset checks: There is a check to validate if the price offset is too large by comparing the price difference between the expected price and the Chainlink price. However, there is no validation to ensure that the fetched price is valid (e.g., non-zero and positive) before using it in the offset check.

3. Stale price feed validation: Although the code includes a check for stale data by comparing the updated timestamp of the feed with the current block’s timestamp, there is still no check to ensure that the price itself is a valid positive value.

Impact

1. Underflows and Overflow Risks: If the price returned is zero or negative, using it in unsigned integer (uint) operations can cause underflows, where the value wraps around and results in unexpected and potentially catastrophic outcomes.

2. Incorrect Contract Behavior: Invalid or unvalidated price data can lead to incorrect contract logic, such as faulty pricing of tokens or faulty conditions being met in calculations that rely on prices. This may lead to users being charged incorrect amounts, or the contract behaving in unintended ways.

3. Security Breaches: Attackers may be able to manipulate prices or exploit the lack of validation to trigger unwanted outcomes, like bypassing certain logic, triggering unintentional operations, or taking advantage of price discrepancies to drain funds or affect the system’s integrity.

Recommendations

1. Validate Prices: Implement checks to ensure that the price fetched from the Chainlink oracle is non-zero, positive, and falls within expected bounds. A validation should be performed before any use in arithmetic operations or conditions that depend on the price.

2. Check for Negative Prices: Specifically check that the fetched price is not negative. Since uint types cannot represent negative values, any negative prices would cause unexpected behavior.

3. Stale Price Feed: While the code already includes a check for stale price feeds, consider enhancing the checks to also validate the price itself before using it in critical contract logic.

4. Add Boundaries and Thresholds: Implement additional checks to verify that the price returned by Chainlink is within a reasonable range, to ensure that extreme price values don't affect the contract.

5. Unit and Integration Tests: Test for edge cases where prices might be zero, negative, or extremely high, ensuring that these scenarios are properly handled and the contract behaves securely under all conditions.

[Low-1] Missing zero address check in initializer

Summary

Initializer functions in contracts are used to set important parameters or addresses. Failing to check for the zero address (0x0000000000000000000000000000000000000000) can lead to unintended behavior. This address is commonly used as a default or unset value, and using it in critical state variables or parameters could result in permanent loss of assets or broken functionality. To prevent such issues, a require(targetAddress != address(0), "Address cannot be zero") check should be added in the initializer functions. This ensures that important addresses are properly set, and the integrity of the contract is maintained.

Num of instances: 1

Vulnerability Details

In the initialize function, critical addresses like _orderHandler, _liquidationHandler, and others are being set. However, there is no check to ensure that none of these addresses are the zero address (0x0000000000000000000000000000000000000000). If any of these addresses are accidentally set to the zero address, it could result in unintended contract behavior, such as failed function calls or loss of funds.

For example:

If _orderHandler is set to the zero address, the contract may try to interact with it but fail, causing errors or unexpected behavior.
Similarly, setting addresses such as _gExchangeRouter or _gmxRouter to the zero address could break the functionality of routing or trading logic.
These scenarios would compromise the functionality and security of the contract, and the issue could go unnoticed unless specifically tested for.

Impact

1. Loss of funds: If critical addresses (such as handler addresses) are set to the zero address, interactions that depend on these addresses will fail, leading to the loss of assets or funds.

2. Broken functionality: Other components of the contract that rely on these addresses for proper operation might stop functioning as expected. This could break important features like order processing, liquidations, or data access.

3. Security risks: Attackers might exploit this oversight by passing the zero address to critical functions, disabling parts of the contract’s functionality or causing a denial of service (DoS).

Recommendations

1. Zero address check: Add a require(targetAddress != address(0), "Address cannot be zero") check before setting any important address in the initializer. This will prevent addresses from being set to the zero address unintentionally.

1. Thorough testing: Implement thorough testing to ensure that no zero address is used in critical functions, especially during contract initialization. This will help catch such issues early in development or testing phases.

2. Additional checks: Consider implementing additional validation for other types of inputs that might affect the contract’s operation, such as non-zero balances or specific minimum values.

[Low-2] No access control on receive/payable fallback

Summary

Without access control on receive() or payable fallback functions in a contract, anyone can send Ether (ETH) to the contract's address. If there's no way to withdraw those funds defined within the contract, any Ether sent, whether intentionally or by mistake, will be permanently stuck. This could lead to unintended loss of funds. Implementing proper access control ensures that only authorized addresses can interact with these functions. A potential resolution could involve adding access control mechanisms like Ownable or specific permission requirements, or creating a withdrawal function accessible only to the contract's owner, thus preventing the unintentional loss of funds.

Vulnerability Details

In this contract, there is a receive() function defined as follows:

This function allows the contract to receive Ether, but there are no restrictions on who can send Ether to it. This means that anyone can send Ether to the contract’s address, potentially leading to funds becoming stuck if no mechanism is in place to withdraw or manage these funds. Furthermore, the contract lacks any functionality to restrict access to these funds or to withdraw them, potentially resulting in an irreversible loss of Ether.

If the contract’s owner or another authorized party needs to access the funds in the contract, the absence of any access control leaves the contract vulnerable to misuse or accidental loss of funds.

Impact

1. Unintended loss of funds: Anyone can send Ether to the contract, potentially leading to the loss of Ether if there is no withdrawal mechanism.

2. Lack of control: Since there are no access controls, malicious actors or unintended transactions could lead to funds being trapped in the contract with no way to recover them.

3. Security risk: If the contract is intended to hold funds temporarily, but no access control is implemented, malicious actors could exploit the contract by flooding it with Ether, which could lead to a denial of service (DoS).

Recommendations

1. Implement access control: Add access control to the receive() function by restricting who can send Ether to the contract. You can use OpenZeppelin's Ownable contract or any other access control mechanism.
   For example, using Ownable:

This would restrict Ether transfers to the contract to only the contract’s owner.

1. Create a withdrawal function: Implement a withdrawal function that allows authorized users (e.g., the owner) to withdraw any Ether sent to the contract. This ensures that any Ether sent accidentally or intentionally can be retrieved by the rightful party.

1. Consider event logging: Optionally, consider emitting events when Ether is received or withdrawn to provide an audit trail of any Ether interactions with the contract.

2. Testing: Test the contract thoroughly to ensure that access control and withdrawal functionality are working as expected, and that no Ether is accidentally or maliciously sent to the contract without proper mechanisms in place to retrieve it.

[Low-3] Chainlink price feed decimals not checked

Summary

Chainlink price feeds are commonly used in decentralized applications (dApps) to fetch reliable price data from oracles. These price feeds often return data with a fixed number of decimal places. However, if the number of decimals is not correctly accounted for in your smart contract, it could lead to calculation errors, such as underflows or incorrect price comparisons. This can happen when the price feed returns data with a different number of decimal places than expected by the contract.

For instance, if a price feed returns a price with 8 decimals, but the contract expects 18 decimals (common with ETH and many ERC-20 tokens), it can result in unexpected behavior or financial discrepancies. To prevent such issues, it's important to retrieve the number of decimals used by the price feed via the decimals() function in the Chainlink AggregatorV3Interface and properly adjust calculations accordingly.

This ensures that the contract operates correctly with any price feed, regardless of the number of decimals used, and prevents errors in price-related operations.

Num of instances: 1

Vulnerability Details

In this contract, the Chainlink price feed is accessed, but the number of decimals used by the feed is not checked before performing calculations:

The issue here is that the decimals used by the price feed are not dynamically retrieved and taken into account. Without checking the decimals of the price feed, calculations may be performed assuming a fixed number of decimals (such as 18), leading to inaccurate results and potential financial errors.

Impact

1. Incorrect Price Calculations: If the number of decimals returned by the price feed does not match the contract’s assumptions, this can cause incorrect calculations, such as overestimating or underestimating the price.

2. Financial Discrepancies: Incorrectly scaled price data can result in wrong amounts being transferred or incorrect decision-making in the contract, which could lead to financial losses or unintended behavior.

3. Compatibility Issues: The contract may fail to properly interact with price feeds that use a different decimal normalization. This could cause issues with other DeFi protocols or price aggregators that use varying decimal places.

Recommendations

1. Check Decimals Dynamically: Use the decimals() function provided by the Chainlink price feed to retrieve the actual number of decimals used by the feed. Once you have this information, scale the price data accordingly to match your contract’s expectations.

For example:

1. Adjust Calculations: When performing calculations involving price feeds, ensure that the decimal precision is accounted for, and prices are properly scaled before being used in arithmetic operations.

2. Testing: Thoroughly test the contract with various price feeds that may have different decimal places. Ensure that the contract behaves correctly across multiple price feed sources and that all calculations are consistent.

3. Use Price Feed Standards: Follow best practices in handling price feeds as outlined by Chainlink and other DeFi developers to ensure that your contract remains resilient to discrepancies in price feed data.