foxb868 - 'fdivUp()' function is used to calculate the target balance, which can result in rounding errors that an attacker can exploit to drain funds from the contract.

[sherlock-admin]

foxb868

high

'fdivUp()' function is used to calculate the target balance, which can result in rounding errors that an attacker can exploit to drain funds from the contract.

Summary

_redeemYT() function uses the fdivUp() function to calculate the target balance, this calculation can result in rounding errors that can be exploited by an attacker to drain funds from the contract.

Vulnerability Detail

+-----------------------+
|                       |
|       Codebase        |
|                       |
+-----------+-----------+
            |
            | Uses
            |
+-----------v-----------+
|                       |
|      redeemYT()       |
|                       |
+-----------+-----------+
            |
            | Uses
            |
+-----------v-----------+
|                       |
|       fdivUp()        |
|                       |
+-----------+-----------+
            |
            | Uses
            |
+-----------v-----------+
|                       |
|    _reweightLScale()   |
|                       |
+-----------------------+

The _redeemYT() function in the "codebase" uses the fdivUp() function to calculate the target balance, which can result in rounding errors that can be exploited by an attacker to drain funds from the contract. Specifically, the issue occurs when calculating the number of tokens to transfer from the adapter to the user when the "PTs" are at a loss and "YTs" had their principal cut to help cover the shortfall. In this case, the function calculates the remaining balance of YT tokens using the _reweightLScale() function, which returns a value that can be rounded down due to the division by integers, however, the fdivUp() function is used instead of fdiv() to round up the result, which can cause an error in favor of the attacker.

Here is how an attacker can exploit and steal funds from the contract:

1. An attacker deposits 1,000 tokens into the contract and receives 1,000 YT tokens.
2. The adapter's target balance is 10,000 tokens and the current total supply of YT tokens is 10,000.
3. The "PTs" suffer a loss, and the adapter's target balance is reduced to 5,000 tokens.
4. The current total supply of YT tokens is still 10,000, so the contract uses the _reweightLScale() function to calculate the remaining balance of YT tokens: (1,000 + 0) / ((1,000 / 10,000) + (0 / 5,000)) = 5,000.
5. The user redeems their 1,000 YT tokens for the remaining balance of PT tokens using the _redeemYT() function. The function calculates the number of tokens to transfer as follows: 1,000.fdivUp(10,000 / 5,000) = 500. However, the correct number of tokens to transfer should be 1,000, since the contract still holds 5,000 PT tokens.

6. The attacker repeats this process until they drain all the PT tokens from the contract.

         -----------------------------------------
         |                                       |
         |             CONTRACT                  |
         |                                       |
         -----------------------------------------
           |                                  |
     1,000 tokens                          1,000 YT tokens
           |                                  |
           |                                  |
        attacker                           contract
   
                   2. Current Total Supply: 10,000 YT Tokens
                   Adapter's Target Balance: 10,000 tokens
   
           |                                  |
           |                                  |
               (PTs suffer loss)         Adapter's Target Balance reduced to 5,000 tokens
           |                                  |
           |                                  |
   
                   3. Current Total Supply: 10,000 YT Tokens
                     Contract uses the `_reweightLScale()` function 
                     to calculate the remaining balance of YT tokens: 
                     (1,000 + 0) / ((1,000 / 10,000) + (0 / 5,000)) = 5,000
   
           |                                  |
           |                                  |
       user redeems                    5,000 PT Tokens
       1,000 YT tokens                       
           |                                  |
           |                                  |
   
              5. The function calculates the number of PT tokens to transfer as follows: 
              1,000.fdivUp(10,000 / 5,000) = 500
              However, the correct number of tokens to transfer should be 1,000, 
              since the contract still holds 5,000 PT tokens. 
   
           |                                  |
           |                                  |
        attacker                          contract
   
        6. Attacker repeats this process until they drain all the PT tokens from the contract.

   Impact

   The vulnerability could allow attackers to exploit rounding errors in the _redeemYT() function to steal funds from the contract.

Code Snippet

https://github.com/sense-finance/sense-v1/blob/82abac25404d83b7aefaaeb46631f1d050dc4a4e/pkg/core/src/Divider.sol#L459 https://github.com/sense-finance/sense-v1/blob/82abac25404d83b7aefaaeb46631f1d050dc4a4e/pkg/core/src/Divider.sol#L459-L489

Tool used

Manual Review

Recommendation

The _redeemYT() function should use the fdiv() function instead of fdivUp() to calculate the number of tokens to transfer.

[jparklev]

Hi foxb868, we appreciate the description you've provided above. Unfortunately, we were unable to follow the reasoning here and are requesting clarification if you don't mind. Specifically, we're having trouble with the following assumptions:

The _redeemYT() function in the "codebase" uses the fdivUp() function to calculate the target balance, which can result in rounding errors that can be exploited by an attacker to drain funds from the contract. Specifically, the issue occurs when calculating the number of tokens to transfer from the adapter to the user when the "PTs" are at a loss and "YTs" had their principal cut to help cover the shortfall.

It is our understanding that fdivUp is only used to calculate rewards with notify, not to determine how many Target tokens to transfer

In this case, the function calculates the remaining balance of YT tokens using the _reweightLScale() function, which returns a value that can be rounded down due to the division by integers, however, the fdivUp() function is used instead of fdiv() to round up the result, which can cause an error in favor of the attacker.

We don't believe _reweightLScale is used for this purpose, but only to update lscale values for issuance, and for when somebody receives YTs

[hrishibhat]

Closing based on Sponsor comments.