Summary

The getLockIdsByOwner function in the SDLPool contract contains a potential issue related to the use of the assert statement to check the equality of lockIdsFound and lockCount. This assert statement is meant to ensure an internal invariant but is currently used for an external condition. This choice could lead to less user-friendly behavior in case of external calls and could impact the overall reliability of the contract.

Along with this, assert use is discouraged in production and is only preffered in test environments mostly.

Vulnerability Details

In the getLockIdsByOwner function:

The assert statement checks whether lockIdsFound is equal to lockCount. This check is meant to ensure an internal invariant, but the function is marked as external, meaning it can be called externally. If an external caller triggers the assert condition to be false, the entire transaction will revert, consuming all gas and rolling back state changes.

Impact

The impact of this vulnerability includes:

1. Potential loss of Gas:

   • External callers invoking the function might face a situation where the entire transaction fails due to the assert condition, consuming all gas. This is less user-friendly and could result in unexpected behavior for users interacting with the contract.

2. Possible Inconsistency in State:

   • If external conditions lead to a discrepancy between lockIdsFound and lockCount, the function could revert the entire transaction, potentially leaving the contract in an inconsistent state.

Tools Used

Manual Review

Recommendations

Replace assert with require in getLockIdsByOwner:

• Consider replacing the assert statement with a require statement for external conditions. This change will provide a more graceful handling of conditions and a better user experience.