search

code-423n4 / 2021-11-malt-findings

0 stars 0 forks source link

setSampleMemory counter set to right value? #193

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

gpersoon

Vulnerability details

Impact

The function setSampleMemory of MovingAverage.sol takes the modulo of counter with the new value of _sampleMemory: "counter = counter % _sampleMemory;"

Suppose: counter =15 ; sampleMemory=10 and _sampleMemory=12 Then: counter = counter % _sampleMemory ==> 3, which means processing will continue at position 3.

However I think it should use: counter = counter % sampleMemory, so it will continue at position 5

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/d3f6a57ba6694b47389b16d9d0a36a956c5e6a94/src/contracts/MovingAverage.sol#L424-L442

function setSampleMemory(uint256 _sampleMemory) external onlyRole(ADMIN_ROLE, "Must have admin privs")  {
  ...
    if (_sampleMemory > sampleMemory) {
      ...
      counter = counter % _sampleMemory;
    } else {
   }
    sampleMemory = _sampleMemory;
}

Tools Used

Recommended Mitigation Steps

Doublecheck the theory above and if you agree: change

 counter = counter % _sampleMemory;

to

 counter = counter %  sampleMemory;
GalloDaSballo commented 2 years ago

The sponsor agrees with the finding so we'll allow