Revenue stream split can be bypassed

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L90

Vulnerability details

The Spigot.claimRevenue function allows (anyone) to claim revenue tokens from the spigot (push and pull payments) and escrows them for the owner to withdraw later.

The revenue is automatically split between the treasury and escrow according to the settings in SpigotState.settings[revenueContract].ownerSplit.

However, the SpigotLib.claimRevenue function does not check whether the revenueContract is a valid revenue contract address.

Impact

Anyone (e.g. the borrower or the SpigotState.treasury owner) can call Spigot.claimRevenue with an arbitrary revenueContract address, forcing a revenue stream split of 0% to the escrow and 100% to the treasury.

Proof of Concept

modules/spigot/Spigot.sol#L74

function claimRevenue(address revenueContract, address token, bytes calldata data)
    external nonReentrant
    returns (uint256 claimed)
{
    return state.claimRevenue(revenueContract, token, data);
}

utils/SpigotLib.sol#L90

If the Spigot contract receives its revenue via push payments and the SpigotLib.claimRevenue function is called with an arbitrary revenueContract address, 0% of the revenue stream will be escrowed and **100%**will be immediately transferred to the treasury address.

function claimRevenue(SpigotState storage self, address revenueContract, address token, bytes calldata data)
    external
    returns (uint256 claimed)
{
    claimed = _claimRevenue(self, revenueContract, token, data);

    // splits revenue stream according to Spigot settings
    uint256 escrowedAmount = claimed * self.settings[revenueContract].ownerSplit / 100;
    // update escrowed balance
    self.escrowed[token] = self.escrowed[token] + escrowedAmount;

    // send non-escrowed tokens to Treasury if non-zero
    if(claimed > escrowedAmount) {
        require(LineLib.sendOutTokenOrETH(token, self.treasury, claimed - escrowedAmount));
    }

    emit ClaimRevenue(token, claimed, escrowedAmount, revenueContract);

    return claimed;
}

Tools Used

Manual review

Recommended mitigation steps

Consider asserting in the SpigotLib.claimRevenue function that the revenueContract address is a valid revenue contract.

[c4-judge]

dmvt marked the issue as duplicate of #169

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt marked the issue as duplicate of #169

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt marked the issue as duplicate of #70

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #119