add a chainID check to pm ticket auxData validation to prevent cross-chain replay attacks

Open
#366 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
solidity

Research direction

Start by locating requireValidTicketAuxData() and getCreationRoundAndBlockHash(), then check the contract's Solidity compiler version and existing ticket validation tests. Confirm how _auxData is encoded before updating validation for the 96-byte layout and chain ID; done means cross-chain ticket data is rejected while valid tickets continue to pass.

Written by the indexing model from the issue text.

Description

enhancement pm streamflow

The istanbul upgrade for ethereum includes EIP1344 which introduces a new assembly chainid opcode allowing a contract to query the chainID of the network it is running on.

We'd need to upgrade to solidity 0.5.12

We should add validation for it to prevent cross-chain replay attacks whereby a message signed off-chain could be re-used on a different chain if the same keys are used.

This can be done by adding a new 32-byte word to the _auxData field for tickets.

We can then add a method that verifies the chain ID
e.g.

    // _auxData format:
    // Bytes [0:31] = creationRound
    // Bytes [32:63] = creationRoundBlockHash
    // Bytes [64:95] = chainID
    function isValidChainID(bytes memory _auxData) internal pure returns (bool) {
        uint256 chainID;
        uint256 ticketChainID;
        assembly {
            chainID := chainid()
            ticketChainID := mload(add(_auxData), 96)
        }
        return chainID == ticketChainID;
    }
}

which would then be called in requireValidTicketAuxData()

        require(
            isValidChainID(_auxData),
            "ticket chainID is not compatible with the network being used"
        );

** extra **
We currently check the length of _auxData in the getCreationRoundAndBlockHash(_auxData) helper, I would suggest moving this at the top of requireValidTicketAuxData() instead so we don't have to validate the length in the functions called in that body. We'd also have to increase the length check to 96 bytes if we're adding a word.

    function requireValidTicketAuxData(bytes memory _auxData) internal view {

        // _auxData format:
        // Bytes [0:31] = creationRound
        // Bytes [32:63] = creationRoundBlockHash
        // Bytes [64:95] = chainID
        require(
            _auxData.length == 96,
            "invalid length for ticket auxData: must be 96 bytes"
        );
...
}
Dominant language
JavaScript
Stars
155
Forks
50
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from livepeer/protocol

All issues in livepeer/protocol

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.