`BufferStored` could be larger than `BufferCap` after `_setBufferCap()`

[CloudEllie]

Judge @jack-the-pug has assessed the second item in QA Report #64 as Medium risk. The relevant finding follows:

Impact

In RateLimited.sol BufferCap should be the upper bound of BufferStored, However in _setBufferCap() it calls _updateBufferStored() before replacing the old BufferCap. If old BufferCap is larger than new BufferCap, BufferStored could be larger than the new BufferCap. This is still a low risk, since BufferStored won't be directly used in `RateLimited.sol. But it could be a potential threat in the future.

Proof of Concept

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/utils/RateLimited.sol#L141

    function _setBufferCap(uint256 newBufferCap) internal {
        _updateBufferStored();

        uint256 oldBufferCap = bufferCap;
        bufferCap = newBufferCap;

        emit BufferCapUpdate(oldBufferCap, newBufferCap);
    }

Recommended Mitigation Steps

Fixing the order can solve this problem.

    function _setBufferCap(uint256 newBufferCap) internal {
        uint256 oldBufferCap = bufferCap;
        bufferCap = newBufferCap;

       _updateBufferStored();

        emit BufferCapUpdate(oldBufferCap, newBufferCap);
    }

[CloudEllie]

Duplicate of #29

[jack-the-pug]

Confirmed!