search

code-423n4 / 2024-05-predy-validation

0 stars 0 forks source link

Incomplete Reset of Nested Structures in Smart Contracts Risks Data Integrity #669

Closed c4-bot-6 closed 3 months ago

c4-bot-6 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/types/GlobalData.sol#L69

Vulnerability details

Impact

When using the delete keyword on a variable of a nested structure, only the top-level fields are reset to their default values. Nested fields within the structure remain unchanged, which can lead to unintended behavior and potential vulnerabilities. This issue is present in the finalizeLock function within the GlobalDataLibrary library of the PredyPool contract.

Proof of Concept

The delete globalData.lockData; line only resets the top-level fields of the lockData structure to their default values. Nested fields within the structure, such as quoteReserve, baseReserve, locker, and pairId, are not explicitly reset, which can lead to residual data remaining in these fields.

delete globalData.lockData;

Example Scenario:

  1. Before Deletion: globalData.lockData contains the following data: 
    globalData.lockData.quoteReserve = 1000;
globalData.lockData.baseReserve = 2000;
globalData.lockData.locker = 0x123...;
globalData.lockData.pairId = 1;
  2. After Deletion: Only the top-level lockData field is reset, but nested fields may retain their previous values.
function finalizeLock(GlobalDataLibrary.GlobalData storage globalData)
    internal
    returns (int256 paidQuote, int256 paidBase)
{
    paidQuote = settle(globalData, true);
    paidBase = settle(globalData, false);

    // This line only resets the top-level fields, nested fields may remain unchanged
    delete globalData.lockData;
}

Tools Used

Manual

Recommended Mitigation Steps

Ensure that each nested field within the lockData structure is explicitly deleted before deleting the outer structure.

function finalizeLock(GlobalDataLibrary.GlobalData storage globalData)
    internal
    returns (int256 paidQuote, int256 paidBase)
{
    paidQuote = settle(globalData, true);
    paidBase = settle(globalData, false);

    // Explicitly delete nested fields first
    delete globalData.lockData.quoteReserve;
    delete globalData.lockData.baseReserve;
    delete globalData.lockData.locker;
    delete globalData.lockData.pairId;

    // Then delete the outer structure
    delete globalData.lockData;
}

Assessed type

Other