Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consensusTakeReceipt performs incorrect receiptId check #6

Open
hats-bug-reporter bot opened this issue Sep 2, 2024 · 1 comment
Open

consensusTakeReceipt performs incorrect receiptId check #6

hats-bug-reporter bot opened this issue Sep 2, 2024 · 1 comment
Labels
bug Something isn't working Invalid - Lead auditor

Comments

@hats-bug-reporter
Copy link

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

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 2, 2024
@0xRizwan
Copy link

0xRizwan commented Sep 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Invalid - Lead auditor
Projects
None yet
Development

No branches or pull requests

1 participant