First Flight #12: Kitty Connect

Severity

Impact: Medium, since the owner can still bridge his NFT

Likelihood: High, since it can be performed by almost anyone

Description

_ccipReceive() is a critical function in the CCIP system which is invoked on the receiver contract by the CCIP router.

The issue is found in the passing of msg.sender as the _sender parameter of the onlyAllowlisted modifier from KittyBridgeBase.

However, in order for the function not to revert, msg.sender can only be the address of the CCIP router because of the following public function exposed by CCIPReceiver (that KittyBridge inherits) that has the onlyRouter modifier:

This makes possible for anyone to mint a cat NFT by directly calling the router without passing from KittyConnect:bridgeNftToAnotherChain(), the only function that is responsible for checking that the caller actually owns the NFT.

POC

Refer to the following diagram: https://drive.google.com/file/d/1JolPeIiFjYYS1Um8TkUWhMSTg0oa2IWj/view?usp=sharing

Recommendations

To always make sure that the NFT is owned and burn by the catOwner on the source chain, we must add proper access control to the functions listed below:

1. protect KittyBridge:bridgeNftWithData() to only allow the KittyConnect contract to call it

2. Change the second parameter of the onlyAllowListed as follows to pass the actual sender of the message (which will be the KittyBridge on the source chain)

Of course, the KittyBridge on the destination chain, must allow-list the address of the KittyBridge on the source chain in his allowlistedSenders mapping.

The any2EvmMessage.sender is safely set by the CCIP router on the source chain:

Also refer to the following ChainLink CCIP official example:
https://docs.chain.link/ccip/tutorials/programmable-token-transfers-defensive#tutorial