Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Description
Brief/Intro
Anyone can call the BridgeRouter Recieve function with malicious data to transfer funds. But this is only viable when theres an adapterID that is 0 which according to the test file had zero as an adapterID( https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/test/bridge/BridgeRouterSpoke.test.ts#L41C2-L44C62 ). This gives the possibility of having an adapterID with zero as id.
Vulnerability Details
There are two mapping updated when adding adapter which is idToAdapter and adapterToId . In a case that there is an adapter whose id is 0 then idToAdapter of 0 will be mapping to an address . in the recieve function of bridge router
function receiveMessage(Messages.MessageReceived memory message) external payable override {
// check if caller is valid adapter
IBridgeAdapter adapter = IBridgeAdapter(msg.sender);
uint16 adapterId = adapterToId[adapter];
if (!isAdapterInitialized(adapterId)) revert AdapterUnknown(adapter);
// check if haven't seen message
if (seenMessages[adapterId][message.messageId]) revert MessageAlreadySeen(message.messageId);
// convert handler to address type (from lower 20 bytes)
address handler = Messages.convertGenericAddressToEVMAddress(message.handler);
// add msg.value to user balance if present
if (msg.value > 0) {
bytes32 userId = _getUserId(Messages.decodeActionPayload(message.payload));
balances[userId] += msg.value;
}
// store message as seen before call to handler
seenMessages[adapterId][message.messageId] = true;
// call handler with received payload
try BridgeMessenger(handler).receiveMessage(message) {
// emit message received as suceeded
emit MessageSucceeded(adapterId, message.messageId);
} catch (bytes memory err) {
// don't revert so GMP doesn't revert
// store and emit message received as failed
failedMessages[adapterId][message.messageId] = message;
emit MessageFailed(adapterId, message.messageId, err);
}
}
which anyone can call it makes sure that the msg.sender is an adapter by checking if adapterToId of the msg.sender will return a valid id. but since all invalid msg.sender will return 0 as id and 0 is already mapped to the a valid address it will pass and go on . this will lead to the attacker being the provide of malicious data . This can be critical if it really happen on mainnet as it can transfer protocol funds to the attacker. pointing it out because it is implemented on the test file so it possible on mainnet.
Impact Details
This could lead to loss of funds and i would suggest 0 id should revert if one tries to add it as adapterid
