Anyone can change fee amount and receiver

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/royalty-vault/contracts/RoyaltyVaultFactory.sol#L52-L76

Vulnerability details

Impact

RoyaltyVaultFactory - everyone can set the platform fee and recipient to themselves and receive all the tokens users pay to themselves and steal all the vault's tokens (add the onlyOwner to these functions) - an attacker can change the fee to 99.99% to steal almost all the tokens as the fee receiver (the splitter requires amount > 0)

/**
 * @dev Set Platform fee for collection contract.
 * @param _platformFee Platform fee in scaled percentage. (5% = 200)
 * @param _vault vault address.
 */
function setPlatformFee(address _vault, uint256 _platformFee) external {
    IRoyaltyVault(_vault).setPlatformFee(_platformFee);
}

/**
 * @dev Set Platform fee recipient for collection contract.
 * @param _vault vault address.
 * @param _platformFeeRecipient Platform fee recipient.
 */
function setPlatformFeeRecipient(
    address _vault,
    address _platformFeeRecipient
) external {
    require(_vault != address(0), "Invalid vault");
    require(
        _platformFeeRecipient != address(0),
        "Invalid platform fee recipient"
    );
    IRoyaltyVault(_vault).setPlatformFeeRecipient(_platformFeeRecipient);
}

Tools Used

VS Code and Remix

Recommended Mitigation Steps

add the onlyOwner modifier to these functions

[sofianeOuafir]

duplicate of #10

[deluca-mike]

Duplicate of #131, so Grouping this with the warden’s QA report, #131