Matic

RootChainManager.sol lines 250-255 the exitHash is determined by three factors, one of which is inputDataRLPList[8], the branch mask, i.e. the path in the receipt merkle patricia trie from the root to the corresponding burning transaction receipt. The path is in bytes and translated to hex by _getNibbleArray() in MerklePatriciaProof.sol. There are 2 cases depending whether the path in hex has odd or even length. In case on line 150 we accept every beginning nibble except 1,3 and erase the first two nibble in the hex array, which means two arrays 2055 and 4055 would produce the same _getNibbleArray() to bypass `verify`, but they produce different exitHash so one can exit more than once.

Matic wants to enable users to send transactions to matic network without changing the network in their wallet so the chainId is the same for both child chain and main chain. This opens an attack vector when a user wants to send a transaction only to the child chain, then the attacker can submit the signed transaction to the other chain against that user’s will. The requires the same nonce, which can happen, and the same contract address on both chains to have an exploitable function, for example when the two contracts are deployed from the same address.

The function `numItems()` calculates the number of elements in an `RLPItem` representing a list, so it should check whether the input indeed represents a list. For example it would return the length of a string. On the other hand adding a `require` check is not necessary and would only cost more gas, since the only place this function is used is in `toList()` and the aforementioned condition is already checked. The exhibit is here just for completeness in case the `RLPReader.sol` library is expanded and the function `numItems()` is used in additional functions.

`require` can be used to check for conditions and throw an exception if the condition is not met, in which case the error message provided by the developer will appear. This is why a very descriptive error message is needed.

In the constructors of `ChildToken` contracts the depositor role is assigned to the contract’s creator. The depositor is an important role as it is the only address that is authorized to deposit the tokens from root contract to child contract. From the natspec it is evident that only the `ChildChainManager` contract should possess this role and this contract does not deploy the `ChildToken` contracts so one would needs some transactions to pass over the depositor rights.

In several functions of `RLPReader` library we check that `item.len` is not zero. Indeed any RLP encoding (even of empty string or empty array) is non-empty, this means we should check this condition in the function `toRlpItem()` to exclude this case before hand.

According to RLP encoding specification the special value “false” is encoded as “0x80”, whereas in current implementation of the function “toBoolean” it is “0x00”.

The subtraction `blockNumber - startBlock` is unsafe and can cause integer underflow, but we believe this cannot be exploited in any way as it would certainly cause the Merkle proof verification to fail.

The IChildToken interface contains only the deposit function and is inherited in every Child Token contract. Every Child Token contains an additional withdraw function which is essential because it enables token withdrawal from the child chain to the main chain so we believe this function belongs to the general pattern as well, i.e. IChildToken interface.

The value in storage `childToRootToken[childToken]` is looked up twice in exit function. Each lookup costs 200 gas each. It would be better to assign this value to a memory variable, because both memory assignment and look up cost 3 gas each.