Public variable `unstreamed` can be smaller than ∑ts.tokens due to `unstreamed` not being updated in `withdraw()`

[code423n4]

Handle

WatchPug

Vulnerability details

unstreamed is a public variable, and it's been actively managed in stake(), updateStreamInternal(). However, since users can also withdraw unstreamed depositToken, the global variable unstreamed should be updated in withdraw() as well.

For example:

1. Alice deposits 10,000 depositToken;
2. Alice withdraws 10,000 depositToken right after step 1.

• Expected Result: unstreamed to be 0;
• Actual Result: unstreamed to be 10,000.

https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L455-L479

function withdraw(uint112 amount) public lock updateStream(msg.sender) {
    require(amount > 0, "amt");

    // checked in updateStream
    // is the stream still going on? thats the only time a depositer can withdraw
    // require(block.timestamp < endStream, "withdraw:!stream");
    TokenStream storage ts = tokensNotYetStreamed[msg.sender];

    require(ts.tokens >= amount, "amt");
    ts.tokens -= amount;

    uint256 virtualBal = dilutedBalance(amount);
    ts.virtualBalance -= virtualBal;
    totalVirtualBalance -= virtualBal;
    depositTokenAmount -= amount;
    if (!isSale) {
        _burn(msg.sender, amount);
    } else {
    }

    // do the transfer
    ERC20(depositToken).safeTransfer(msg.sender, amount);

    emit Withdrawn(msg.sender, amount);
}

Recommendation

Change to:

function withdraw(uint112 amount) public lock updateStream(msg.sender) {
    require(amount > 0, "amt");

    // checked in updateStream
    // is the stream still going on? thats the only time a depositer can withdraw
    // require(block.timestamp < endStream, "withdraw:!stream");
    TokenStream storage ts = tokensNotYetStreamed[msg.sender];

    require(ts.tokens >= amount, "amt");
    ts.tokens -= amount;
    unstreamed -= amount;

    uint256 virtualBal = dilutedBalance(amount);
    ts.virtualBalance -= virtualBal;
    totalVirtualBalance -= virtualBal;
    depositTokenAmount -= amount;
    if (!isSale) {
        _burn(msg.sender, amount);
    } else {
    }

    // do the transfer
    ERC20(depositToken).safeTransfer(msg.sender, amount);

    emit Withdrawn(msg.sender, amount);
}

[0xean]

dupe of #118