sherlock-audit / 2024-08-flayer-judging

2 stars 0 forks source link

utsav - `_createCheckpoint()` will run twice in ProtectedListings:createListings() but it is meant to run only once #740

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

utsav

Medium

_createCheckpoint() will run twice in ProtectedListings:createListings() but it is meant to run only once

Summary

_createCheckpoint() will run twice in ProtectedListings:createListings() but it is meant to run only once

Vulnerability Detail

When a tokenId of a collection is protectedListed for the first time then it calls _createCheckpoint() and it stores the checkPointIndex

 function createListings(CreateListing[] calldata _createListings) public nonReentrant lockerNotPaused {
//
            // Update our checkpoint for the collection if it has not been done yet for
            // the listing collection.
            checkpointKey = keccak256(abi.encodePacked('checkpointIndex', listing.collection));
            assembly { checkpointIndex := tload(checkpointKey) }
            if (checkpointIndex == 0) {
>               checkpointIndex = _createCheckpoint(listing.collection);
                assembly { tstore(checkpointKey, checkpointIndex) }
            }
//
    }

But the problem is, if we see _createCheckpoint(), it returns the index = 0 for the first time calling, which means tstore will store again 0 at checkpointKey. As result, it will run again but it should not.

function _createCheckpoint(address _collection) internal returns (uint index_) {
//
        // If this is our first checkpoint, then our logic will be different as we won't have
        // a previous checkpoint to compare against and we don't want to underflow the index.
        if (index_ == 0) {
            // Calculate the current interest rate based on utilization
            (, uint _utilizationRate) = utilizationRate(_collection);

            // We don't have a previous checkpoint to calculate against, so we initiate our
            // first checkpoint with base data.
            collectionCheckpoints[_collection].push(
                Checkpoint({
                    compoundedFactor: locker.taxCalculator().calculateCompoundedFactor({
                        _previousCompoundedFactor: 1e18,
                        _utilizationRate: _utilizationRate,
                        _timePeriod: 0
                    }),
                    timestamp: block.timestamp
                })
            );

>           return index_;
//
        }

Impact

It will create wrong checkPoint by running twice

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/ProtectedListings.sol#L134C12-L139C14 https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/ProtectedListings.sol#L539C9-L556C27

Tool used

Manual Review

Recommendation

Return index_++, if the checkpoint is create for the first time of that collection