BenRai - In `PriceTierVesting` there is no check if the Sequenzer for L2s is up when calling the oralce

[sherlock-admin2]

BenRai

medium

In PriceTierVesting there is no check if the Sequenzer for L2s is up when calling the oralce

Summary

Price from oracle on L2s can be invalid/stale if the sequencer is down. This could lead to users being able to claim tokens that they should not be able to claim.

Vulnerability Detail

If the sequencer of the L2s were to go offline the Chainlink oracle may return an invalid/stale price. This could lead to users being able to claim tokens that they should not be able to claim.

For example, if the last reference price recorded by the oracle was above the final tier price, this would have unlocked all tokens to be claimable. If the sequencer goes and in the meantime down the reference price falls below the final tier price , the oracle would still return the high price even though the price is lower now and not all tokens should be claimable.

It should always be checked if the sequencer is up before consuming any data from Chainlink. For more details on L2 Sequencer Uptime Feeds check the Chainlink docs(https://docs.chain.link/data-feeds/l2-sequencer-feeds) specify more details.

Impact

Receivers can claim tokens they should not be able to claim

Code Snippet

https://github.com/sherlock-audit/2023-06-tokensoft/blob/1f58ddb066ab383c416cfcbf95c9902683506e96/contracts/contracts/claim/abstract/PriceTierVesting.sol#L30-L45

Tool used

Manual Review

Recommendation

Include a check if the sequencer is up. If it is down, revert when calling getVestedFraction in PriceTierVesting

[BenRai1]

Escalate

According to the judge, the issue was excluded because PriceTierVesting.sol is out of scope for the contest. Even though PriceTierVesting.sol was not explicitly mentioned as in scope, deriving from the following issue in the Teller contest, (https://github.com/sherlock-audit/2023-03-teller-judging/issues/328), PriceTierVesting.sol is implicitly in scope because PriceTierVestingSale_2_0.sol is in scope and it is inheriting from PriceTierVesting. This means all behaviour of PriceTierVesting that affects PriceTierVestingSale_2_0 should also be in scope.

Therefore in my opinion, this issue should be considered as valid.

[sherlock-admin2]

Escalate

According to the judge, the issue was excluded because PriceTierVesting.sol is out of scope for the contest. Even though PriceTierVesting.sol was not explicitly mentioned as in scope, deriving from the following issue in the Teller contest, (https://github.com/sherlock-audit/2023-03-teller-judging/issues/328), PriceTierVesting.sol is implicitly in scope because PriceTierVestingSale_2_0.sol is in scope and it is inheriting from PriceTierVesting. This means all behaviour of PriceTierVesting that affects PriceTierVestingSale_2_0 should also be in scope.

Therefore in my opinion, this issue should be considered as valid.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[jkoppel]

Duplicate: #22, #38, #48, #218, #229, #38, #163

[Shogoki]

Regarding scoping:

In my opinion every contract in scope should be listed explicitly as in scope. I do get that it might makes sense to have some rules to have this contract implicitly in scope, but it creates uncertainty as it is nowhere documented. I raised this concern in discord here I will leave the final decision on the scope for this on sherlock, as there are good points for both sides, i guess. But the following discussions in discord should also be taken into account:

https://discord.com/channels/812037309376495636/1130514276570906685/1138286182254522531 https://discord.com/channels/812037309376495636/1130514276570906685/1131243195817283604

However this is decided at the end, we should take this as a chance to improve the guidelines for scope in future contests.

Regarding the Issue:

This can be a valid Medium

[hrishibhat]

Result: Medium Has duplicates Since PriceTierVesting is in scope because PriceTierVestingSale_2_0 is in scope. This is a valid medium Agree with the @Shogoki comments that the scoping rules should be improved to avoid such confusion

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• BenRai1: accepted

[cr-walker]

Fixed by https://github.com/SoftDAO/contracts/pull/17

[maarcweiss]

Fixed by creating the L2OracleWithSequencerCheck.sol contract, as the contract that checks whether the sequencer is up.