`ts.tokens` can potentially be reduced more than expected

[code423n4]

Handle

WatchPug

Vulnerability details

In the current implementation, ts.lastUpdate will only be updated when ts.tokens > 0. Thus, ts.lastUpdate can be outdated for an exited user who deposits again.

As a result, by the next time updateStreamInternal() is called, ts.tokens will be reduced more than expected mistakenly.

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

modifier updateStream(address who) {
    // save bytecode space by making it a jump instead of inlining at cost of gas
    updateStreamInternal(who);
    _;
}

function updateStreamInternal(address who) internal {
    ...
    // update users unstreamed balance
    uint32 acctTimeDelta = uint32(block.timestamp) - ts.lastUpdate;
    if (acctTimeDelta > 0 && ts.tokens > 0) {
        // some time has passed since this user last interacted
        // update ts not yet streamed
        ts.tokens -= uint112(acctTimeDelta * ts.tokens / (endStream - ts.lastUpdate));
        ts.lastUpdate = uint32(block.timestamp);
    }
    ...

PoC

Given:

• startTime: 1/1/2022
• endStream: 1/1/2023

1. On 1/1/2022: Alice calls stake() and exit() right after, ts.lastUpdate is 1/1/2022;
2. On 1/12/2022: Alice calls stake(1000e18). At L226, as ts.tokens == 0, ts.lastUpdate remains 1/1/2022;
3. Right after step 2, Alice calls stake(1 wei):
   • Expected Result: ts.tokens remains 1000e18, and almost all of the 1000e18 can be withdrawn;
   • Actual Result: Since ts.lastUpdate is not updated at step 2. At L225, acctTimeDelta will be ~11 mos. Thus, at L229 ts.tokens will be reduced to 83.33e18, and only 83.33e18 can be withdrawn.

Recommendation

Change to:

// update users unstreamed balance
uint32 acctTimeDelta = uint32(block.timestamp) - ts.lastUpdate;
if (acctTimeDelta > 0 && ts.tokens > 0) {
    // some time has passed since this user last interacted
    // update ts not yet streamed
    ts.tokens -= uint112(acctTimeDelta * ts.tokens / (endStream - ts.lastUpdate));
}
ts.lastUpdate = uint32(block.timestamp);

[brockelmore]

duplicate #123