Storage gaps mismanagement may corrupt contract state

[code423n4]

Lines of code

https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/Furnace.sol#L108

Vulnerability details

Impact

Mismanagement of storage gaps in All upgradable contracts within scope may cause a corrupted storage state of a contract that inherits from it, due to shifted storage slots. Link provided (furnace.sol) is one instance, however every upgradable contract has a different sized storage slot.

Proof of Concept

Most every upgradable contract uses a different sized '_gap' for future upgrades.

For example in furnace.sol If the initial number of storage gaps was 47, then the current number should be 47 reserves slots - 5 used slots = 42. If this mistake is made during upgrading of a deployed contract, then storage slots of the contract that inherits from another contract could shift up by an unexcpected amount, which will corrupt the state of the contract.

Recommended Mitigation Steps

It is recomended that you use an identical number of storage gaps in all upgradeable contracts. Also ensure that the number is only decremented when a new state variable is added, there is never a reason for it to increment.

[0xean]

given that gaps aren't a strict requirement and more of a best practice to make things easier / less error prone, I am downgrading this issue to QA.

For more discussion on the topic see - code-423n4/org#55

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as grade-c