Permissions - return values not checked when sending ETH

[code423n4]

Handle

ScopeLift

Vulnerability details

Impact

On lines 85 and 101, ETH is transferred using a .call to an address provided as an input, but there is no verification that the call call succeeded. This can result in a call to emergencyWithdrawGAS or partialWithdrawGAS appearing successful but in reality it failed. This can happen when the provided destination address is a contract that cannot receive ETH, or if the amount provided is larger than the contract's balance

Proof of Concept

Enter the following in remix, deploy the Receiver contract, and send 1 ETH when deploying the Permissions contract. Call emergencyWithdrawGAS with the receiver address and you'll see it reverts. This would not be caught in the current code

pragma solidity ^0.8.0;

contract Receivier{}

contract Permissions {
    constructor() payable {}

    function emergencyWithdrawGAS(address payable destination) external {
        (bool ok, ) = destination.call{value: address(this).balance}('');
        require(ok, "call failed");
    }
}

Tools Used

Remix

Recommended Mitigation Steps

In emergencyWithdrawGAS:

- destination.call{value: address(this).balance}('');
+ (bool ok, ) = destination.call{value: address(this).balance}('');
+ require(ok, "call failed");

And similar for partialWithdrawGAS

[GalloDaSballo]

Agree with the finding, I believe if a developer were to not use safeTransfer we'd rate as medium, so while I believe the impact to be minimal (no composability), I'll keep the severity to medium