Low level calls with solidity version 0.8.14 can result in optimiser bug.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/TokenRegistry.sol#L2
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/libraries/LibDiamond.sol#L2
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L2
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/test/Connext.sol#L2

Vulnerability details

Impact

The protocol is using low level calls with solidity version 0.8.14 which can result in optimizer bug.

Proof of Concept

See POC from Certora

Recommended Mitigation Steps

Consider upgrading to solidity 0.8.15

[ecmendenhall]

This is indeed a gnarly optimizer bug in Solidity 0.8.13 and 0.8.14, but it only occurs under very specific conditions. In order to trigger the bug, the legacy optimizer must be enabled (true for the current project configuration), and the affected assembly blocks must not refer to any local Solidity variables.

The second condition does not appear true for the examples here: every inline assembly block in the linked files for this finding references an outside variable.

[ecmendenhall]

Trying to keep my comments factual, but I would also note that 1) I think this is still worth reporting, and 2) I have reported this myself as an informational finding in other contests, even when there was no evidence of vulnerable assembly blocks.

[LayneHaber]

Agree that this is worth reporting, and we will adopt the suggestions. Think we can lower the severity since it doesn't seem to fit all of the conditions. Perhaps this is more of a QA issue?

[LayneHaber]

connext/monorepo@900de7a

[0xleastwood]

This bug is definitely an interesting one to breakdown. It only affects self-contained assembly code when the yul optimiser is enabled. As a result, memory operations are removed from the block, leading to unexpected handling of memory later in the function. This can introduce some unintended behaviour later on, but this does not affect the connext codebase. Downgrading to QA and merging with #263.

[0xleastwood]

Here is some more context https://blog.soliditylang.org/2022/06/15/inline-assembly-memory-side-effects-bug/