any replica can call handle() in RelayerFeeRouter.sol and claim fees for different transfer ids

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/4dd6149748b635f95460d4c3924c7e3fb6716967/contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol#L135

Vulnerability details

Issue: any replica can call handle if it passes as origin a domain that does not exists and as _sender address(0).

Consequences: can claim fees for different transfer ids.

Affected Code

File: RelayerFeeRouter.sol
130:   function handle(
131:     uint32 _origin,
132:     uint32 _nonce,
133:     bytes32 _sender,
134:     bytes memory _message
135:   ) external override onlyReplica onlyRemoteRouter(_origin, _sender) { // @audit-info [HIGH] 
136:     // parse recipient and transferIds from message
137:     bytes29 _msg = _message.ref(0).mustBeClaimFees();
138: 
139:     address recipient = _msg.recipient();
140:     bytes32[] memory transferIds = _msg.transferIds();
141: 
142:     connext.claim(recipient, transferIds);
143: 
144:     // emit Receive event
145:     emit Receive(_originAndNonce(_origin, _nonce), _origin, recipient, transferIds);
146:   }

Mitigations

check for _router to not be empty (_router != bytes32(0)) inside the _isRemoteRouter() function on BaseConnextFacet

[LayneHaber]

Not sure the best way to handle this but this is the same root cause (though different consequences / places to fix) as #254

[LayneHaber]

https://github.com/connext/nxtp/pull/1450/commits/b2a65a30a03443db6500ca1a56514f40321ce1c0

[0xleastwood]

I don't believe this issue to be valid.

Home.dispatch() sets _sender to msg.sender. This is checked in onlyRemoteRouter as s.remotes[_domain] == _router. Therefore, if _sender is not the remote router, then this will revert.

[0xleastwood]

Additionally, _sender cannot be the zero address because Home.dispatch() will always set this to msg.sender.