Description:Description\
Users can call crossChainProofOfHumanity::transferHumanity(...) to transfer humanity from the current chain in which it exist to another chain. However, due to the implementation of the transferHumanity(...) function, the humanity could be lost
Attack Scenario\
As shown below on L281, after the necessary validation has been done on the humanity to send, the BridgeGateway is meant to call the ICrossChainProofOfHumanity.receiveTransfer() finvtion on the destination chain with the new owner set to msg.sender which is the owner on the origin/source chain.
File: CrossChainProofOfHumanity.sol
250: function transferHumanity(address _bridgeGateway) external allowedGateway(_bridgeGateway) {
251: // Function will require humanity to be claimed by sender, have no pending requests and human not vouching for others at the time
252: (bytes20 humanityId, uint40 expirationTime) = proofOfHumanity.ccDischargeHumanity(msg.sender);
253:
254: CrossChainHumanity storage humanity = humanityData[humanityId];
255:
256: require(block.timestamp > humanity.lastTransferTime + transferCooldown, "Can't transfer yet");
257:
258: // Save state to not require extra updates from receiving chain
259: humanity.expirationTime = expirationTime;
260: @> humanity.owner = msg.sender;
261: humanity.isHomeChain = false;
This is done to prevent overiding the states of the humanity on the receiving chain.
The problem is that, msg.sender which is the owner of the humanity on the current chain may be controlled by a different user on a different chain and as such the ownership of the transfered humanity will be lost to a new owner on the receiving chain.
Revised Code File (Optional)
Modify crossChainProofOfHumanity::transferHumanity(...) to ensure that the owner of the humanity on the receiving chain is controlled by the current owner or its desired address as shown below
File: CrossChainProofOfHumanity.sol
-250: function transferHumanity(address _bridgeGateway) external allowedGateway(_bridgeGateway) {
+250: function transferHumanity(address _bridgeGateway, address recipientChainOwner) external allowedGateway(_bridgeGateway) {
251: // Function will require humanity to be claimed by sender, have no pending requests and human not vouching for others at the time
252: (bytes20 humanityId, uint40 expirationTime) = proofOfHumanity.ccDischargeHumanity(msg.sender);
253:
254: CrossChainHumanity storage humanity = humanityData[humanityId];
255:
256: require(block.timestamp > humanity.lastTransferTime + transferCooldown, "Can't transfer yet");
257:
258: // Save state to not require extra updates from receiving chain
259: humanity.expirationTime = expirationTime;
-260: humanity.owner = msg.sender;
+260: humanity.owner = recipientChainOwner;
261: humanity.isHomeChain = false;
clesaege
commented
3 months ago
Github username: @Audinarey Twitter username: audinarey Submission hash (on-chain): 0xb7842b2d829d45a964a00bf2dd406b62602d8ceb81b01cc9e32a45347e253596 Severity: high
Description: Description\ Users can call
crossChainProofOfHumanity::transferHumanity(...)to transfer humanity from the current chain in which it exist to another chain. However, due to the implementation of the transferHumanity(...) function, the humanity could be lostAttack Scenario\ As shown below on L281, after the necessary validation has been done on the humanity to send, the
BridgeGatewayis meant to call theICrossChainProofOfHumanity.receiveTransfer()finvtion on the destination chain with the new owner set to msg.sender which is the owner on the origin/source chain.This is done to prevent overiding the states of the humanity on the receiving chain.
The problem is that, msg.sender which is the owner of the humanity on the current chain may be controlled by a different user on a different chain and as such the ownership of the transfered humanity will be lost to a new owner on the receiving chain.
Modify
crossChainProofOfHumanity::transferHumanity(...)to ensure that the owner of the humanity on the receiving chain is controlled by the current owner or its desired address as shown below