_getTwav is not correct. It is not considering all last 4 blocks.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Twav/Twav.sol#L33-L43

Vulnerability details

Impact

This will result in invalid deadline to decide the buyout end.

Proof of Concept

/// @notice returns the TWAV of the last 4 blocks /// @return _twav TWAV of the last 4 blocks function _getTwav() internal view returns(uint256 _twav){ if (twavObservations[TWAV_BLOCK_NUMBERS - 1].timestamp != 0) { uint8 _index = ((twavObservationsIndex + TWAV_BLOCK_NUMBERS) - 1) % TWAV_BLOCK_NUMBERS; TwavObservation memory _twavObservationCurrent = twavObservations[(_index)]; TwavObservation memory _twavObservationPrev = twavObservations[(_index + 1) % TWAV_BLOCK_NUMBERS]; _twav = (_twavObservationCurrent.cumulativeValuation - _twavObservationPrev.cumulativeValuation) / (_twavObservationCurrent.timestamp - _twavObservationPrev.timestamp); } }

====> In above codes, only tow indices, _index, _index+1 is considered the twav and return it. This may not be the current calculation. When you check the comments its says like "/// @return _twav TWAV of the last 4 blocks"

Tools Used

Manual verification

Recommended Mitigation Steps

Use appropriate twav calculation methods.

[mundhrakeshav]

It uses a accumulator.

[GalloDaSballo]

Finding is lacking Code / POC and going for High Severity

[HardlyDifficult]

It uses a accumulator.

Agree. The code is storing up to 4 values, no more than once per block. In then uses the newest and oldest of these values for the calculation here - giving you the "TWAV of the last 4 blocks". Closing this as invalid.