code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

Overwriting rdpxAmount Without Checks #2161

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/decaying-bonds/RdpxDecayingBonds.sol#L144

Vulnerability details

Impact

The decreaseAmount function, specifically the line bonds[bondId].rdpxAmount = amount;. This vulnerability allows overwriting the rdpxAmount without any checks or backups, which can result in irreversible data loss.

  function decreaseAmount(
    uint256 bondId,
    uint256 amount
  ) public onlyRole(RDPXV2CORE_ROLE) {
    _whenNotPaused();
    bonds[bondId].rdpxAmount = amount;
  }

Accidental exploitation of this vulnerability can occur when a user mistakenly enters a larger value for the amount parameter than intended. This can happen due to human error or incorrect input validation. For example, if a user intends to decrease the rdpxAmount by 10, but mistakenly enters 100, the vulnerability will be triggered, and the rdpxAmount will be overwritten with the incorrect value.

The ramifications of this vulnerability can be significant:

Data Loss: If the rdpxAmount is overwritten with a value greater than the original amount, it can lead to a loss of data integrity. This can result in incorrect calculations, unexpected behavior, or financial discrepancies. Inconsistent State: The vulnerability can lead to an inconsistent state where the rdpxAmount doesn't accurately reflect the actual amount of the bond. This can cause confusion and make it difficult to track and manage bond amounts accurately. Financial Loss: If the vulnerability is exploited maliciously, an attacker could intentionally overwrite the rdpxAmount with a significantly lower value, causing financial loss to the bondholder or the contract owner.

Proof of Concept

Here's how this vulnerability affects the flow of the contract:

1) The decreaseAmount function is intended to decrease the rdpxAmount of a bond. It can only be called by a role with the RDPXV2CORE_ROLE.

2) The vulnerability lies in the line bonds[bondId].rdpxAmount = amount;. This line directly overwrites the rdpxAmount without any checks or backups.

3) Without checks or backups, there is a risk of irreversible data loss. If the amount provided is greater than the current rdpxAmount, it will result in a decrease that exceeds the original value, potentially causing negative values or unexpected behavior.

To reproduce the vulnerability and demonstrate its impact, you can follow these steps:

1) Deploy the vulnerable contract on a test network or local blockchain. Make sure you have the necessary environment set up to deploy and interact with smart contracts.

2) Call the decreaseAmount function with a bondId and a new amount that is greater than the current rdpxAmount. contractInstance.decreaseAmount(bondId, newAmount);

3) As a result, the rdpxAmount of the specified bond will be overwritten with the new amount, even if it exceeds the original value. This can lead to irreversible data loss or unexpected behavior.

4) You can verify the impact of the vulnerability by checking the updated rdpxAmount for the affected bond using a getter function or by inspecting the contract's state. uint256 updatedAmount = contractInstance.getBondRdpxAmount(bondId);

5) If the updatedAmount is different from the previous value and greater than expected, it confirms that the vulnerability has been successfully reproduced, and the rdpxAmount has been overwritten without any checks or backups.

Tools Used

foundry

Recommended Mitigation Steps

To fix this vulnerability, you can implement the following changes:

Add a check to ensure that the new amount is not greater than the old amount before updating the rdpxAmount.

require(amount <= oldAmount, "New amount is greater than the old amount");

Emit an event to capture the old amount and the new amount before updating the rdpxAmount.

emit BondAmountUpdated(bondId, oldAmount, amount); By adding these checks and emitting the event, you can mitigate the risk of irreversible data loss and provide transparency for changes in rdpxAmount.

Assessed type

Invalid Validation

bytes032 commented 1 year ago

Spam. Same as #1894

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid