Multicall.sol batch calls don't update msg.value

[code423n4]

Handle

jayjonah8

Vulnerability details

Impact

In NestedFactory.sol using Multicall.sol can be dangerous when it has a msg.value inside a loop since the msg.value doesn't update every iteration. This can lead to a user sending ETH one time and it being counted for every iteration. There is a msg.value in create() => _submitOrders => _transferInputTokens() which simply changes ETH for WETH. This _transferInputTokens() function is called alot in NestedFactory.sol by many functions. This can introduce serious bugs in the future as the protocol grows.

Proof of Concept

https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedFactory.sol#L6

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Multicall.sol

https://samczsun.com/two-rights-might-make-a-wrong/

Tools Used

Manual code review

Recommended Mitigation Steps

Remove the Open Zeppelin Multicall.sol functionality since it doesn't seem to be used and can introduce serious future bugs.

[maximebrugel]

Duplicated : #226