re-al-Foundation / rwa-contracts

0 stars 0 forks source link

[RSE-03M] Inexistent Evaluation of Valid Deposit #61

Closed chasebrownn closed 5 months ago

chasebrownn commented 5 months ago

RSE-03M: Inexistent Evaluation of Valid Deposit

Type Severity Location
Logical Fault RevenueStreamETH.sol:L139

Description:

The RevenueStreamETH::depositETH function will not validate that the msg.value being deposited is non-zero, causing a significant discrepancy in the cycles stored. Specifically, it will permit the same cycle to be introduced multiple times which in turn will cause claim as well as expiry operation to account for the same cycle twice incorrectly.

Impact:

If the same cycle is introduced twice to the cycles array, the amount owed for a user as well as amount due for expiry would be accounted twice incorrectly and thus lead to an accounting deficit in the contract that would translate to loss of funds.

Example:

/**
 * @notice This method is used to deposit ETH into the contract to be claimed by shareholders.
 * @dev Can only be called by an address granted the `DEPOSITOR_ROLE`.
 */
function depositETH() payable external {
    require(msg.sender == revenueDistributor, "RevenueStreamETH: Not authorized");

    if (revenue[block.timestamp] == 0) {
        cycles.push(block.timestamp);
        revenue[currentCycle()] = msg.value;
    }
    else {
        /// @dev In the event `depositETH` is called twice in the same second (though unlikely), dont push a new cycle.
        ///      Just add the value to the existing cycle.
        revenue[block.timestamp] += msg.value;
    }

    emit RevenueDeposited(msg.value);
}

Recommendation:

We advise the code to ensure that the msg.value is non-zero, preventing this misbehaviour from arising.

chasebrownn commented 5 months ago

Resolved