hats-finance / OLD-Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

MIT License
0 stars 0 forks source link

consensusTakeReceipt performs incorrect receiptId check #6

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7e8a9483f162fa303cb381cf9da4b303d829648091dfb130ea24a6f065d74114 Severity: medium

Description:

Description

Inside function consensusTakeReceipt the following check is ensured:

    function consensusTakeReceipt(SubcallReceiptKind kind, uint64 receiptId)
        internal
        returns (bytes memory)
    {
=>        if (receiptId == 0) revert InvalidReceiptId();

This check is made to ensure that no incorrect receiptId's are trying to be retrieved whenever calling, for example, takeReceiptDelegate.

The issues arises however, since receiptId starts incrementing from nextReceiptId which is set to 429467296 instead of 0, essentially changing the starting point to a self set number

    nextReceiptId = 4294967296;

This is used whenever a new receiptId is issued, for example, inside delegate

    uint64 receiptId = nextReceiptId++;

but since the check inside consensusTakeReceipt uses 0 it will incorrectly allow any receiptId's above 0 which in fact can never be valid since the receiptId starts incrementing from nextReceiptId

This could end up in allowing fake receipts to be let through

Recommendation

Change the receiptId check inside consensusTakeReceipt to correctly disallow incorrect receiptId's

0xRizwan commented 1 month ago

consensusTakeReceipt() as an internal function is used in consensusTakeReceiptDelegate(), consensusTakeReceiptUndelegateStart() and consensusTakeReceiptUndelegateDone() functions so the receiptId being used as input by these functions would be the starting from nextReceiptId++ i.e ( nextReceiptId = 4294967296 onwards). No fake receipientID can be passed in either of above functions. Therefore, i believe this issue is invalid.