Beanstalk Notion
Beanstalk Notion
📄

Report #13438

Report Date
November 12, 2022
Status
Closed
Payout

An Arithmetic overflow issue in smart contract due to missing Substraction checks

‣
Report Info

Report ID

#13438

Target

https://etherscan.io/address/0x39cdAf9Dc6057Fd7Ae81Aaed64D7A062aAf452fD

Report type

Smart Contract

Impacts

  • Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
  • Arithmetic Underflow issue in smart contract (Out of scope)

Has PoC?

Yes

Bug Description

I found an issue in Fertilizer.sol smart contract in which developer forgot to implement a security check while performing subtraction operations

Link = https://etherscan.io/address/0x39cdAf9Dc6057Fd7Ae81Aaed64D7A062aAf452fD?utm_source=immunefi#code

image

Exact Issue

In Link = https://etherscan.io/address/0x39cdAf9Dc6057Fd7Ae81Aaed64D7A062aAf452fD?utm_source=immunefi#code

At Line 73 you see an __update() function inside this there is a code

image

As shown in Picture of the value of variable stopBpf is less then _Balances[ids[i]] [account].lastBefore

Then output gets Overflows because current solidity version used in this smart contract is ^0.7.0 where Overflows and Underflow issue still exists

Poc Video = https://drive.google.com/file/d/1OQ5E3_CvFDNjd8zsp-ETUxTaB7j0JLza/view?usp=sharing

Impact

  1. Wrong output result we see in variable DeltaBpf this could leads to code break situation too
image
  1. Loss of funds due to this overflow result we see and also harm other functions where this function value is used
  2. Arithmetic Overflow issue in smart contract because this smart contract uses solidity version ^0.7.0 which is still vulnerable to overflow and Underflow issue

Recommendation

Developer should implement a check where stopBpf < _Balances[ids[i]] [account].lastBefore

 Require ( stopBpf < _Balances[ids[i]] [account].lastBefore )

References

OpenZeppelin safemath.sol

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/SafeMath.sol

image

Please review this issue I think it's causing a series Impact on smart contract

Proof of concept

Step 1 : Open the Link = https://etherscan.io/address/0x39cdAf9Dc6057Fd7Ae81Aaed64D7A062aAf452fD?utm_source=immunefi#code

Step 2 : At Line 73 you see an __update() function inside this there is a code

image

As shown in Picture of the value of variable stopBpf is less then _Balances[ids[i]] [account].lastBefore

Then output gets Overflows because current solidity version used in this smart contract is ^0.7.0 where Overflows and Underflow issue still exists

Poc Video = https://drive.google.com/file/d/1OQ5E3_CvFDNjd8zsp-ETUxTaB7j0JLza/view?usp=sharing

Step 3 : Arithmetic Overflow issue in smart contract because this smart contract uses solidity version ^0.7.0 which is still vulnerable to overflow and Underflow issue

BIC Response

After some review, the BIC has determined that this is not a valid bug report because overflow is impossible in this case.

In Line 79 of Fertilizer.sol: stopBpf = min(endBpf, s.bpf)

  • endBpf is the Fertilizer Id
  • s.bpf is the input to __update(...) as bpf is set to IBS(owner()).beansPerFertilizer())

As endBpf is a constant and s.bpf is increasing, we can conclude that stopBpf is constant or strictly increasing.

In line 83: _balances[ids[i]][account].lastBpf = uint128(stopBpf);

As stopBpf for a given id is strictly constant or increasing and _balances[ids[i]][account].lastBpf is set to stopBpf the last time __update(...) was called, it can be concluded that:

  • _balances[ids[i]][account].lastBpf >= stopBpf.

Therefore it is impossible for overflow to occur in the statement: stopBpf - _balances[ids[i]][account].lastBpf

Due to these reasons, we are closing the submission and no reward will be issued.

Immunefi Response

Our finding is that the attack described in this bug report is impossible. This submission was correctly closed and no reward should be given.

TECHNICAL ASSESSMENT:

Here are some more specifics from our technical assessment of the mediation request. After thorough review our team concluded that it is impossible to achieve the overflow.

In the __update() the uint256 deltaBpf = stopBpf - _balances[ids[i]][account].lastBpf; is used to calculate beans value to be emitted in the ClaimFertilizer event. The lastBpf value was set based on the stopBpf value, and for the underflow to occur the stopBpf value must be smaller than the lastBpf value.

This condition is impossible to be met because the stopBpf can only increment overtime or constantly. Therefore the lastBpf value will always be lower than the stopBpf value and therefore, the underflow can't be reached.

Even though the you managed to make the deltaBpf value underflow, it still must go through (https://etherscan.io/address/0x39cdaf9dc6057fd7ae81aaed64d7a062aaf452fd/advanced#code#82) which has safety checks and will result in the reverted transaction.