Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BVR-03M] Inexistent Access Control of Protocol Withdrawals #742

Open
zajck opened this issue Aug 1, 2023 · 0 comments
Open

[BVR-03M] Inexistent Access Control of Protocol Withdrawals #742

zajck opened this issue Aug 1, 2023 · 0 comments
Labels
wontfix This will not be worked on

Comments

@zajck
Copy link
Member

zajck commented Aug 1, 2023

BVR-03M: Inexistent Access Control of Protocol Withdrawals

Type Severity Location
Logical Fault BosonVoucher.sol:L766

Description:

The BosonVoucherBase::withdrawToProtocol function does not apply any access control to its caller, permitting anyone to invoke it and thus cause funds from the contract to be deposited to the protocol.

While the funds will still be owned by the correct sellerId, the BosonVoucherBase contract is capable of being the "purchaser" of a conditional offer in the Boson Protocol system. These conditional offers can impose restrictions based on EIP-20 asset balances that can be compromised by this function in an on-chain race condition.

Impact:

It is presently possible to hijack threshold-based commit authorizations that are performed by the BosonVoucherBase by invoking its BosonVoucherBase::withdrawToProtocol function.

Example:

function withdrawToProtocol(address[] calldata _tokenList) external {
    address protocolDiamond = IClientExternalAddresses(BeaconClientLib._beacon()).getProtocolAddress();
    uint256 sellerId = getSellerId();

    for (uint256 i = 0; i < _tokenList.length; i++) {
        address token = _tokenList[i];
        if (token == address(0)) {
            uint256 balance = address(this).balance;
            IBosonFundsHandler(protocolDiamond).depositFunds{ value: balance }(sellerId, token, balance);
        } else {
            uint256 balance = IERC20(token).balanceOf(address(this));
            IERC20(token).approve(protocolDiamond, balance);
            IBosonFundsHandler(protocolDiamond).depositFunds(sellerId, token, balance);
        }
    }
}

Recommendation:

We advise the code to apply proper access control and ensure that the function can only be called by the OwnableUpgradeable::owner of the contract.

@zajck zajck added the wontfix This will not be worked on label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant