sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

simon135 - If block range it big and adl dosnt use currentblock in the order it will cause issues #216

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

simon135

medium

If block range it big and adl dosnt use currentblock in the order it will cause issues

Summary

if block range is a big range and since adl order usescache.minOracleBlockNumbers[0] there can be an issue of the block not being in range because of the littlest block with max block being a lot bigger and adl wont happen and the protocol will get bad debt

Vulnerability Detail

since the adl order updateBlock is cache.minOracleBlockNumbers[0] the block can be behind the range check and fail and the protocol can end up in bad debt with the tokens price declining

Impact

bad debt

Code Snippet

        cache.key = AdlUtils.createAdlOrder(
            AdlUtils.CreateAdlOrderParams(
                dataStore,
                eventEmitter,
                account,
                market,
                collateralToken,
                isLong,
                sizeDeltaUsd,
                cache.minOracleBlockNumbers[0]
            )

Tool used

Manual Review

Recommendation

make it chain.currentBLock

xvi10 commented 1 year ago

doesn't seem to be a valid issue, cache.minOracleBlockNumbers[0] is from oracleParams which is provided by the ADL keeper, the ADL keeper should use the latest prices to execute ADL

IllIllI000 commented 1 year ago

@xvi10 I know that the order keepers are intended to eventually be only semi-trusted. Is that not the case for ADL keepers - they'll always be fully trusted?

xvi10 commented 1 year ago

i think the impact is minimized if there is at least one honest ADL keeper, if all ADL keepers are dishonest it is possible for ADLs to continually occur using older prices

IllIllI000 commented 1 year ago

It sounds like ADL keepers are not fully trusted and therefore this is an unlikely, but possible risk