Summary

Due to the lack of access control on the stopFunding function, anyone can monitor the blockchain, identify a program with an active funding process, and maliciously call the stopFunding function to stop the program, resulting in improper fund allocation.
I know you have already described it
https://github.com/sherlock-audit/2024-11-superfluid-locking-contract-HeYuan-33?tab=readme-ov-file#q-please-discuss-any-design-choices-you-made
But can you be sure that the participant is not an attacker?

Root Cause

Vulnerable code:

2024-11-superfluid-locking-contract-HeYuan-33/fluid/packages/contracts/src/FluidEPProgramManager.sol

Line 294 in e0d5af5

function stopFunding(uint256 programId) external {

As we can see, the stopFunding function is only marked as external, meaning it can be called externally. This allows an attacker to freely stop a program, leading to improper fund allocation.
Internal pre-conditions
No response

External pre-conditions

The program ID is obtained by the attacker, and it is an actively running program.

Attack Path

The attacker obtains an actively running program ID through blockchain monitoring.
The attacker calls the stopFunding function, causing the program to be forcibly stopped.

Impact

An actively running program can be prematurely terminated.
Maliciously trigger related fund distribution and compensation operations.
If an attacker calls stopFunding, it will delete the program’s information, and it cannot be recovered permanently.

PoC

No response

Mitigation

Add a check to verify if the caller is a participant in the project, to prevent non-participants from calling the function.No matter what, adding a check for the caller is never a mistake.

这次的审计报告提交的很少,基本没有什么漏洞,这次主要是去看了合约的逻辑实现,其实一些精度计算,四舍五入的那些地方没有去仔细看,就忽略了这个漏洞,虽然这次提交又是被判为无效,但是也是知道了首先阅读审计开始前的要求,还有就是对于那些权限很松的注意一下抢跑攻击,比如这次的claim函数,就是和pool建立联系,但是攻击者也可以提前知道用户的交易,然后抢先去提交交易,让用户建立不了链接,以下是正确的报告。还是的学习怎么写poc测试。

抢跑攻击

FluidLocker can be blocked from connecting to a pool

Summary

For FluidLockers to get money streamed into them they need to be connected to the distribution pools. This connection is established in claim function. However an attacker can observe locker transactions and call FluidEPProgramManager::UpdateUserUnits before user transactions go through. This will mark the signatures as used and update FluidLocker units without connecting the locker to the distribution pool.

Root Cause

FluidLocker’s claim function updates locker units, verifies signatures and also establishes the connection between the locker and pool. https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/1fa5f86024be5f269e1a0898b1f939f1d4cce149/fluid/packages/contracts/src/FluidLocker.sol#L155C1-L171C6.
This connection will enable lockers to reflect their token balances on the event of a distribution, otherwise some address or the locker needs to manually claim for the locker.
However an attacker can observe user’s claim and call FluidEPProgramManager::UpdateUserUnits before user transactions go through. This will mark the signatures as used and update FluidLocker units without connecting the locker to the distribution pool.
https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/1fa5f86024be5f269e1a0898b1f939f1d4cce149/fluid/packages/contracts/src/EPProgramManager.sol#L119C1-L149C6 (Note that the _poolUpdate of this function is overridden by FluidEPProgramManager’s _poolUpdate).

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

User calls claim
Attacker front runs the call with FluidEPProgramManager::UpdateUserUnits with same parameters

Impact

Although users can claim for the locker at any point, this is out of the ordinary flow and will be unexpected for the users, thus can lead to denial of funds for some time.

PoC

No response

Mitigation

Locker’s connection to the pool should be seperated from claims.

四舍五入的问题,先除后乘,会导致精度损失

When calculating in the _getUnlockingPercentage function, 540 was mistakenly used instead of 540 days for calculation. As a result, users can unlock all their funds earlier without paying penalties.

Summary

When calculating in the _getUnlockingPercentage function, 540 was mistakenly used instead of 540 days for calculation. As a result, users can unlock all their funds earlier without paying penalties.

Root Cause

In FluidLocker.sol#L388, 540 was mistakenly used instead of 540 days for calculation.
In FluidLocker.sol#L379-L381, since the minimum return of _getUnlockingPercentage is 269731 even if it is unlocked using the minimum unlocking period of 7 days(It is assumed that the accuracy problem in the calculation has been fixed.). This is greater than 10000. so it will result in an overflow of L381. This means that the user can’t use the vest form of unlocking. This forces the user to use _instantUnlock and pay an 80% penalty.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Users have to pay an 80% penalty to unlock it. This has resulted in user losses.

PoC

For simplicity, I directly changed the _getUnlockingPercentage function to public and fixed the error due to precision issues first. The code is as follows:

function _getUnlockingPercentage(uint128 unlockPeriod) public pure returns (uint256 unlockingPercentageBP) {  
unlockingPercentageBP = (
_PERCENT_TO_BP
* (
// ((80 * _SCALER) / Math.sqrt(540 * _SCALER)) * (Math.sqrt(unlockPeriod * _SCALER) / _SCALER)
(80 * _SCALER) * (Math.sqrt(unlockPeriod * _SCALER)) / Math.sqrt(540 * _SCALER)
+ 20 * _SCALER
)
) / _SCALER;
}

poc(put in FluidLocker.t.sol):

function test_getUnlockingPercentage() external virtual {  
FluidLocker(address(aliceLocker))._getUnlockingPercentage(7 days);
FluidLocker(address(aliceLocker))._getUnlockingPercentage(540 days);
}

Resault:

[PASS] test_getUnlockingPercentage() (gas: 22807)
Traces:
[22807] FluidLockerTest::test_getUnlockingPercentage()
├─ [13073] BeaconProxy::_getUnlockingPercentage(604800 [6.048e5]) [staticcall]
│ ├─ [2308] UpgradeableBeacon::implementation() [staticcall]
│ │ └─ ← [Return] FluidLocker: [0x426eeFE8AF33482cA9F3ED139b1991984468926E]
│ ├─ [2974] FluidLocker::_getUnlockingPercentage(604800 [6.048e5]) [delegatecall]
│ │ └─ ← [Return] 269731 [2.697e5]
│ └─ ← [Return] 269731 [2.697e5]
├─ [4055] BeaconProxy::_getUnlockingPercentage(46656000 [4.665e7]) [staticcall]
│ ├─ [308] UpgradeableBeacon::implementation() [staticcall]
│ │ └─ ← [Return] FluidLocker: [0x426eeFE8AF33482cA9F3ED139b1991984468926E]
│ ├─ [2956] FluidLocker::_getUnlockingPercentage(46656000 [4.665e7]) [delegatecall]
│ │ └─ ← [Return] 2353510 [2.353e6]
│ └─ ← [Return] 2353510 [2.353e6]
└─ ← [Stop]

Mitigation

    unlockingPercentageBP = (  
        _PERCENT_TO_BP  
            * (  
  •                ((80 * _SCALER) / Math.sqrt(540 * _SCALER)) * (Math.sqrt(unlockPeriod * _SCALER) / _SCALER)
    
  •                (80 * _SCALER) * (Math.sqrt(unlockPeriod * _SCALER)) / Math.sqrt(540 days * _SCALER)  
                      + 20 * _SCALER  
              )  
      ) / _SCALER;
    

计算错误,导致固定的比例,这篇报告真的受膜拜

Invalid vest unlock flow rate calculations in FluidLocker::_vestUnlock(…) leads to recepients paying much higher tax rates than intended

Summary

The FluidLocker contract allows users to lock FLUID tokens, and then at specific unlockPeriods to unlock them. When unlocking, users can choose to do it either instantly, and pay an 80% tax fee to stakers, or use a vesting scheme, where funds are flown to the user, using a Fontaine, based on the unlock period chosen. However, with the current logic in FluidLocker::_vestUnlock(…), there is no difference if a recipient unlocks with unlockPeriod = 0 or unlockPeriod = 540 days as there will be a constant 80% tax.

Root Cause

In FluidLocker::_getUnlockingPercentage(…) invalid calculations are being carried out, leading to the function always producing a result of 2000, no matter what unlockPeriod is given. Because of this, the resulting unlockFlowRate will always be capped at 20% meaning that recipients who unlock at the max period will pay a constant tax of 80% to stakers, as if they unlocked at the very beginning.

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

Locker A’s owner unlocks tokens for Alice with an unlock period of 0.
Alice pays 80% tax rate to instantly receive her tokens.
Locker A’s owner unlocks tokens for Bob with an unlockPeriod of 540 days.
Bob should pay 0 % tax, however due to improper calculations, he ends up paying 80% tax as well.,
Impact
FluidLocker::unlock(…) recipients who choose to unlock using the vesting option pay higher taxes than intended, thus receiving fewer tokens than what they should be.

PoC

The following test can be added in FluidLocker.t.sol and ran with forge test –mt test_getUnlockingPercentage -vv

uint256 private constant _SCALER = 1e18;  

uint256 private constant _PERCENT_TO_BP = 100;

function _getUnlockingPercentage(uint128 unlockPeriod) internal pure returns (uint256 unlockingPercentageBP) {
unlockingPercentageBP = (
_PERCENT_TO_BP
* (
((80 * _SCALER) / Math.sqrt(540 * _SCALER)) * (Math.sqrt(unlockPeriod * _SCALER) / _SCALER)
+ 20 * _SCALER
)
) / _SCALER;
}

// Add this test function to your FluidLockerTest contract
function test_getUnlockingPercentage() public {
// Create a helper function to access the internal function

// Test different periods
uint128 sevenDays = 7 days;
uint128 thirtyDays = 30 days;
uint128 ninetyDays = 90 days;
uint128 oneEightyDays = 180 days;
uint128 fiveFortyDays = 540 days;

uint256 sevenDaysPercent = _getUnlockingPercentage(sevenDays);
uint256 thirtyDaysPercent = _getUnlockingPercentage(thirtyDays);
uint256 ninetyDaysPercent = _getUnlockingPercentage(ninetyDays);
uint256 oneEightyDaysPercent = _getUnlockingPercentage(oneEightyDays);
uint256 fiveFortyDaysPercent = _getUnlockingPercentage(fiveFortyDays);

console.log("7 days unlock percentage:", sevenDaysPercent);
console.log("30 days unlock percentage:", thirtyDaysPercent);
console.log("90 days unlock percentage:", ninetyDaysPercent);
console.log("180 days unlock percentage:", oneEightyDaysPercent);
console.log("540 days unlock percentage:", fiveFortyDaysPercent);
}

And the output is:

Ran 1 test for test/FluidLocker.t.sol:FluidLockerTest
[PASS] test_getUnlockingPercentage() (gas: 19376)
Logs:
7 days unlock percentage: 2000
30 days unlock percentage: 2000
90 days unlock percentage: 2000
180 days unlock percentage: 2000
540 days unlock percentage: 2000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 11.22ms (886.54µs CPU time)

Ran 1 test suite in 159.53ms (11.22ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Mitigation
Divide the current _getUnlockingPercentage calculations into batches to ensure proper truncation:

index f31ad92..f39a89a 100644

--- a/fluid/packages/contracts/src/FluidLocker.sol  
+++ b/fluid/packages/contracts/src/FluidLocker.sol
@@ -382,13 +382,14 @@ contract FluidLocker is Initializable, ReentrancyGuard, IFluidLocker {
}

function _getUnlockingPercentage(uint128 unlockPeriod) internal pure returns (uint256 unlockingPercentageBP) {
- unlockingPercentageBP = (
- _PERCENT_TO_BP
- * (
- ((80 * _SCALER) / Math.sqrt(540 * _SCALER)) * (Math.sqrt(unlockPeriod * _SCALER) / _SCALER)
- + 20 * _SCALER
- )
- ) / _SCALER;
+ // First calculate sqrt(unlockPeriod/_MAX_UNLOCK_PERIOD)
+ uint256 periodRatio = Math.sqrt((unlockPeriod * _SCALER) / _MAX_UNLOCK_PERIOD);
+
+ // Scale it to the range [20, 100]
+ uint256 percentage = 20 + ((80 * periodRatio) / Math.sqrt(_SCALER));
+
+ // Convert to basis points (multiply by 100)
+ unlockingPercentageBP = percentage * 100;
}

With the above change, the test output looks like this:

Ran 1 test for test/FluidLocker.t.sol:FluidLockerTest
[PASS] test_getUnlockingPercentage() (gas: 18470)
Logs:
7 days unlock percentage: 2900
30 days unlock percentage: 3800
90 days unlock percentage: 5200
180 days unlock percentage: 6600
540 days unlock percentage: 10000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 11.90ms (1.07ms CPU time)

Ran 1 test suite in 160.39ms (11.90ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

就是要对于计算,比如,先除后乘就会导致一些精度的损失,或者是计算太大,还有一种的实现方式就是写测试文件