Submitted by 0xRajeev, also found by pauliax
Zero-address checks are in general a best-practice. However, addLiquidity() and removeLiquidity() are missing zero-address checks on router and recipient addresses respectively.
addLiquidity() on Eth transfers will update the zero index balance and get logged as such in the event without the amount getting accounted for the correct router.
For ERC20 assets, token.transfer() generally implements this check but the Eth transfer using transferEth() does not have this check and calls addr.call(value), which will lead to burning in the case of removeLiquidity().
The checks may be more important because assetID is 0 for Eth. So a router may accidentally use 0 values for both assetID and router/recipient.
There is also a missing zero-address check on sendingChainFallback which is relevant for Eth transfers in cancel(). The comment on L178 indicates the need for this but the following check on L179 ends up checking receivingAddress instead (which is also necessary). See issue page for referenced code.
Recommend adding zero-address checks.
sanchaymittal (Connext) confirmed and patched:
