Manager Manual Review Findings

MRE-01M: Unsafe Length Cast

Type	Severity	Location
Mathematical Operations		Manager.sol:L43

Description:

The Manager::batchCall function will fail to encode a call result that is greater than type(uint32).max in length as it will downcast the length member unsafely.

Impact:

While a call containing a result output greater than type(uint32).max in length is infeasible in traditional blockchains, it would still cause the code to corrupt its encoded result thus potentially affecting what the result of consequent calls would supposedly be.

Example:

Copy
36bytes memory callResult = CallLib.callFunc(
37  target,
38  data[offset:offset + callDataLength],
39  value
40);
41
42offset += callDataLength;
43result = abi.encodePacked(result, uint32(callResult.length), callResult);

Recommendation:

We advise the code to perform a safe casting operation from uint256 to uint32 by ensuring that callResult.length is less-than-or-equal-to the maximum value a uint32 data type can hold (i.e. type(uint32).max).

To note, Solidity's built-in safe arithmetic in pragma versions 0.8.X onwards does not cover casting operations.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The 0xPhase team opted to instead yield an overflow message whenever the callResult length exceeded the maximum of a uint32 variable, appending it to the result sequence. Given that a casting overflow is no longer possible, we consider this exhibit adequately alleviated.