Treasury rDPX reserves can be manipulated to get discounted bonds

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L894-L933 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1156-L1199 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reserve/RdpxReserve.sol#L85-L87

Vulnerability details

Impact

When an user issue a bond for the first time user needs to call bond function with tokenId 0. When a new bond occurs the discount will be calculated according to treasury rDPX reserves. The more the rDPX in the treasury bigger the discount will be. However, the discount has no caps, if the discount is big enough it will not be possible to make new bonds because of the underflow in the calculateBondCost function.

Proof of Concept

Assume the following values: bondDiscountFactor = 5e5 (In tests that's the value its set) Math.sqrt(IRdpxReserve(addresses.rdpxReserve).rdpxReserve()) = 15_00 1e18; (15K rDPX) RDPX_RATIO_PERCENTAGE = 25 1e8; (default value from contract) rDPXPriceInETH = 0.00793399 * 1e8; (8 decimal scaled, thats the current value as of writing this report)

When calculating the discount following code is executed: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1163-L1177

bondDiscount = 6123724356 = 61.237 which is lesser than 100e8 so that check will not revert https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1167

However, the function will revert because of the underflow happening in this line: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1170 (25 * 1e8) - (6123724356 / 2) = -0.00000....3 hence, underflow.

So there are no way to create a bond. The '_amount' variable is not going to change it aswell. Rdpx reserves must need to decrease which only can decrease with the bond interactions so its not possible or the bondDiscountFactor needs to be lowered down by admin. So anytime this happens admin needs the adjust bondDiscountFactor manually. This is clearly not an intended behaviour. The best would be to set a minimum and maximum discount.

Another aspect and potential attack scenario can be created with this same root finding which could make this issue a high finding, let's do an example for that too:

Assume rDPX reserves has 9K rDPX this time. Say a whale want to create "1000" bond. If whale would call the bond function with "_amount" as "1000" and assume the "reLP" and "putOptionsRequired" are set to false to make our calculations easier, the amounts that the whale needs to give to core contract will be calculated as: 1616.98 rDPX 512.8 WETH

now, if whale airdrops 1000 rDPX to reserve contract reserve contracts rDPX balance will be 10K. In that case if whale try to create the bond with 1000 the amounts will be calculated as follows: 0 rDPX 500 WETH

As seen the whale airdropped 1K rDPX and in exchange he saved 1616.98 rDPX and 12.8 WETH. Assuming the price we declared above, 1rDPX = 0.00793399WETH

cost of the attack = 1000 0.00793399 = 7.93WETH profit of the attack = 1616.98 0.00793399 + 12 = 24.82WETH net profit of the attack = 24.82 - 7.93 = 16.91WETH

Remix snapshots of calculations of rdpxRequired, wethRequired and discountFactor When reserves are 9K and the "_amount" is 1000

When reserves are 10K and the "_amount" is 1000

Tools Used

Manual, Desmos, Remix

Recommended Mitigation Steps

Do not rely on ERC20 balanceOf() function when calculating rDPX reserves. Put maximum and minimum discount if bondDiscount / 2 > RDPX_PERCENTAGE

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as duplicate of #2209

[c4-pre-sort]

bytes032 marked the issue as duplicate of #2049

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[c4-judge]

GalloDaSballo marked the issue as grade-b