sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

Avci - Chainid logic is Unvarying! #379

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Avci

medium

Chainid logic is Unvarying!

Summary

because Chainid cannot change, if a chain id of a chain changes one day it can be dangerous because of issues like replay attack

Vulnerability Detail

the chain id logic is implemented so there are no options for changing it or getting it external.

Impact

if the chain id doesn't update it can cause replay attacks in chain forks when the chain id of chain is changed.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Game.sol#L59


// array of chainIds e.g [10, 100, 1000];
  uint32[] public chainIds;

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Game.sol#L128

Tool used

Manual Review

Recommendation

look at the open zeppelin explanation about updating chain id to prevent replay attacks during chain id changes https://docs.openzeppelin.com/contracts/3.x/api/drafts#EIP712-_domainSeparatorV4-- The encoding specified in the EIP is very generic, and such a generic implementation in Solidity is not feasible, thus this contract does not implement the encoding itself.

//Returns the domain separator for the current chain.

_domainSeparatorV4()