DelegateToken force to expire the L2 sequencer goes down and bot can call rescind and withdraw fast when L2 sequencer goes up

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L339 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L353

Vulnerability details

Impact

Bot can rescind and withdraw after the L2 sequecer goes down and up

Proof of Concept

The delegate protocol intended to deploy to EVM chain, including L2 such as optimism and arbitrum

The delegate token implements the function below:

   /**
     * @notice Allows the principal token owner or any approved operator to extend the expiry of the delegation rights.
     * @param delegateTokenId The ID of the rights being extended.
     * @param newExpiry The absolute timestamp to set the expiry
     */
    function extend(uint256 delegateTokenId, uint256 newExpiry) external;

    /**
     * @notice Allows the delegate owner or any approved operator to return a delegate token to the principal rights holder early, allowing the principal rights holder to redeem
     * the underlying token(s) early. Allows anyone to forcefully return the delegate token to the principal rights holder if the delegate token has expired
     * @param delegateTokenId Which delegate right to rescind
     */
    function rescind(uint256 delegateTokenId) external;

    /**
     * @notice Allows principal rights owner or approved operator to withdraw the underlying token once the delegation rights have either met their expiration or been rescinded.
     * Can also be called early if the caller is approved or owner of the delegate token (i.e. they wouldn't need to
     * call rescind & withdraw), or approved operator of the delegate token holder
     * "Burns" the delegate token, principal token, and returns the underlying tokens to the caller.
     * @param delegateTokenId id of the corresponding delegate token
     */
    function withdraw(uint256 delegateTokenId) external;

However, the rescind and withdraw does not have access control after the expiry

when the L2 Sequencer goes down to delegate token is forced to expire,

For example, the recent optimism bedrock upgrade cause the sequencer not able to process transaction for a hew hours

https://cryptopotato.com/optimism-bedrock-upgrade-release-date-revealed/

Bedrock Upgrade According to the official announcement, the upgrade will require 2-4 hours of downtime for OP Mainnet, during which there will be downtime at the chain and infrastructure level while the old sequencer is spun down and the Bedrock sequencer starts up.

Transactions, deposits, and withdrawals will also remain unavailable for the duration, and the chain will not be progressing. While the read access across most OP Mainnet nodes will stay online, users may encounter a slight decrease in performance during the migration process.

In Arbitrum

https://thedefiant.io/arbitrum-outage-2

and

https://beincrypto.com/arbitrum-sequencer-bug-causes-temporary-transaction-pause/

Ethereum layer-2 (L2) scaling solution Arbitrum stopped processing transactions on June 7 because its sequencer faced a bug in the batch poster. The incident only lasted for an hour.

when the sequencer goes down and up

the bot can call rescind and withdraw and burn the principle token directly fast and leave user no time to extend the expiry deadline

Tools Used

Manual Review

Recommended Mitigation Steps

chainlink has a sequencer up feed

https://docs.chain.link/data-feeds/l2-sequencer-feeds

consider integrate the up time feed and give user extra time to extend the expiration when sequencer goes up after goes down

also, access control should be added to rescind and withdraw even after expiry

Assessed type

Timing

[c4-sponsor]

0xfoobar (sponsor) disputed

[0xfoobar]

If there's going to be blockchain downtime you should extend before the blockchain goes down, lmfao

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

Not convinced this has any impact, leaving as informational

[GalloDaSballo]

NOTE: yet to rate QA for this Warden

[c4-judge]

GalloDaSballo marked the issue as grade-b

[JeffCX]

I think the impact is when sequencer goes down, it leave user no time to extend the expiration deadline

and the sequencer can go down because of bug so the user cannot be informed before

https://beincrypto.com/arbitrum-sequencer-bug-causes-temporary-transaction-pause/

but I highly respect judge's expertise and will respect judge's final decision on severity!

[0xfoobar]

This is a ridiculous report, a core axiom of smart contracts is that the underlying blockchain is live. It is trivial and obvious and almost insulting to the reader's intelligence to report that "a function cannot be called if the blockchain is dead". Why are we even discussing this? Extending needs to be done ahead of time, if not then the PT owner can trivially recreate a new DT and give to the old holder.

[JeffCX]

Please do not be mad sir,

srry I see this is can be done

if not then the PT owner can trivially recreate a new DT and give to the old holder.

Agree this submission is not valid :)

[GalloDaSballo]

Technically speaking the sequencer going down will queue the tx in the queue or in the mailbox Those will be re-peated in order once the L2 comes back online This means that the observation is technically incorrect as the queue and time would actually be re-inforced once the L2 re-starts