search

code-423n4 / 2022-05-backd-findings

0 stars 0 forks source link

Minus before addition -> underflow risk (But reverted due to solidity 0.8) #172

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L574

Vulnerability details

Impact

totalKeeperPoolWeight = totalKeeperPoolWeight - currentUInts256[key] + pendingUInts256[key];

If currentUInts256[key] > totalKeeperPoolWeight it is clearly reverted due to underflow.

Proof of Concept

totalKeeperPoolWeight = totalKeeperPoolWeight - currentUInts256[key] + pendingUInts256[key];

Obviously minus currentUInts256[key] before addition pendingUInts256[key]. If currentUInts256[key] > totalKeeperPoolWeight it is clearly reverted due to underflow.

But if you plus before minus, it never get underflow even if currentUInts256[key] > totalKeeperPoolWeight

But still underflow if currentUInts256[key] > totalKeeperPoolWeight + pendingUInts256[key]

Tools Used

Scan code by eye

Recommended Mitigation Steps

totalKeeperPoolWeight = totalKeeperPoolWeight + pendingUInts256[key] - currentUInts256[key];

pendingUInts256[key] will be added to totalKeeperPoolWeight first, greater value resist underflow error.

chase-manning commented 2 years ago

totalKeeperPoolWeight will always be greater than currentUInts256[key]

GalloDaSballo commented 2 years ago

I believe the finding to have validity, however, in lack of a POC, with the sponsor disputing, I can't help but downgrade to QA.

Please provide a POC showing how the misconfiguration can actually happen to ensure your findings can withstand basic scrutiny