Open hats-bug-reporter[bot] opened 1 month ago
@party-for-illuminati why this issue is invalid?
@party-for-illuminati why this issue is invalid?
It is how it is by design.
"Malicioud relayer can disrupt the deposit and withdrawal process completly." please provide a PoC
@party-for-illuminati why this issue is invalid?
It is how it is by design.
"Malicioud relayer can disrupt the deposit and withdrawal process completly." please provide a PoC
we are checking this in depth.
Github username: -- Twitter username: -- Submission hash (on-chain): 0x07a8c324cc7925fee2d1a4588ecf79c8dbc6468b57d4d9fa23a77a37250f6ccd Severity: medium
Description: Description
In the current implementation, owner has the power to set or update the relayer. This can be see in the abstract contract
AllowedRelayers
.AllowedRelayers.sol#L27-L33
There is modifier implemented to check whether the caller is relayer or not.
onlyRelayer
Please not the above check will work only if
relayersWhitelistEnabled
is enabled and then it check for relayer in the mapping.when we see the function
toggleRelayersWhitelistEnabled
, this toggle can be update anytime by the owner. so , if it is set as false, theonlyRelayer
will allow anyone act as relayer.This can be problamatic in may cases where some of the criticial functions are depending on the relayer modifier.
For example, lets see the
ackTransaction
function in BitCoinProver contract.Case 1:
BitcoinProver.sol#L263-L277
At the end of the function, it makes the callback function with relayer provided callback address. malicioud relayer could misuse this callback function with its own callback structure.
case 2:
VaultBitcoinWallet.sol#L328-L344
It can finalise any data without checking if the valid data or not inside the _serializers mapping.
Attack Scenario\ Malicioud relayer can disrupt the deposit and withdrawal process completly.
Attachments
it would be better to remove the
relayersWhitelistEnabled
in theonlyRelayer
modifier.