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

The same message can be sent twice in a single block, make them non block unique. #86

Open
hats-bug-reporter bot opened this issue Feb 7, 2024 · 1 comment
Labels
invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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

Description:
Description
Messages will be non block unique and have the same message identifier when they are sent on the same block. This can be done by sending
the same messaages through seperate transactions on the same block. This will make the messages to be non block unique and they
will have the same message identifier.
https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/IncentivizedMessageEscrow.sol#L173

Attack Scenario
The same message can be sent twice in a single block by using multiple trasactions to execute them.

Proof of Concept (PoC) File

Add this tests to MessageIdentifier.t.sol
and run forge test on each of them.


    function test_notUnique_identifier_block_11() public {
        vm.roll(11);
        IncentiveDescription storage incentive = _INCENTIVE;
        (, bytes32 messageIdentifier) = escrow.submitMessage{value: _getTotalIncentive(_INCENTIVE)}(
            _DESTINATION_IDENTIFIER,
            _DESTINATION_ADDRESS_THIS,
            _MESSAGE,
            incentive
        );

        assertEq(messageIdentifier, bytes32(0xeaa2656c806ede225c7826a7d7f26fbc0f3ba4c918a54ed06a04842f76fef24b));
    }

    function test2_notUnique_identifier_block_11() public {
        vm.roll(11);
        IncentiveDescription storage incentive = _INCENTIVE;
        (, bytes32 messageIdentifier) = escrow.submitMessage{value: _getTotalIncentive(_INCENTIVE)}(
            _DESTINATION_IDENTIFIER,
            _DESTINATION_ADDRESS_THIS,
            _MESSAGE,
            incentive
        );

        assertEq(messageIdentifier, bytes32(0xeaa2656c806ede225c7826a7d7f26fbc0f3ba4c918a54ed06a04842f76fef24b));
    }

Poc File attached below

Potential fix

Verify that the same message can't be sent twice on the same block through different transactions.

Files:

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Feb 7, 2024
@reednaa
Copy link
Collaborator

reednaa commented Feb 7, 2024

Both tests are run in seperate and clean vms.

@reednaa reednaa added invalid This doesn't seem right and removed bug Something isn't working labels Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant