Dria

Swan
NFTHardhat
21,000 USDC
View results
Submission Details
Severity: low
Invalid

Uncheck the `marketParameters` in `getCurrentMarketParameters()`

"The given code snippet reveals a critical logical vulnerability in the getCurrentMarketParameters() function where no bounds checking is performed on the array marketParameters. This vulnerability can be exploited by an attacker when the array is empty, leading to an underflow attack that can cause disruption and return invalid data.

https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/SwanManager.sol#L69

Here is a clear step-by-step description of how to exploit this vulnerability:

Step-by-step Exploitation of the Vulnerability

  1. Preconditions for Exploit:

    • The marketParameters array must be empty. This condition could initially exist if the contract is newly deployed and the array hasn't been populated yet.

    • The attacker must be able to call public or external view functions on the contract.

  2. Calling the Vulnerable Function:

    • The attacker calls the getCurrentMarketParameters() function, which attempts to access the last element of the marketParameters array by using the expression marketParameters[marketParameters.length - 1].

  3. Result of the Call:

    • If marketParameters is empty, marketParameters.length will be 0.

    • The code then attempts to access marketParameters[-1] due to the underflow where 0-1=-1. In Solidity, accessing an array with a negative index leads either to a runtime error (reversion) or may result in unexpected behavior depending on compiler version and runtime environment.

Exploitable Outcome

  • Service Disruption: Any external system or interface relying on getting valid market parameters via this function will fail, leading to disruptions in service or faulty outputs, especially if they don't handle failures gracefully.

  • Invalid Output: Depending on the contract environment and Solidity settings, this could potentially output incorrect, uninitialized, or default values if the access does not outright revert, introducing grave data integrity problems.

Recommended Fix

To mitigate this vulnerability, the function should be updated to include a check if the marketParameters array is empty before attempting to access its last element. Here's a simple fix:

function getCurrentMarketParameters() public view returns (SwanMarketParameters memory) {
require(marketParameters.length > 0, ""The marketParameters array is empty."");
return marketParameters[marketParameters.length - 1];
}

This added require statement ensures that there’s at least one element in the array before accessing it, thus avoiding underflow and ensuring that getCurrentMarketParameters() only returns valid results or reverts with a clear error message when no data is available."

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.