dimulski - A malicious actor can manipulate the distribution of points #85
Comments
github-actions
bot
added
the
Excluded
|
1 comment(s) were left on this issue during the judging contest. 0xmystery commented:
|
sherlock-admin3
changed the title
|
Escalate, |
You've created a valid escalation! To remove the escalation from consideration: Delete your comment. You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final. |
|
When the malicious actor first deposits 1 wei, |
@mystery0x The only thing first deposit does for this issue is it sets the Also, there is no concern for |
|
You guys would need to trace the flow carefully/manually. First off, |
|
@AtanasDimulski do you have the counterarguments? @mystery0x could you please share, if theoretically it's infeasible, where's the mistake in the report's POC? Doesn't it prove that it's indeed possible? |
|
@WangSecurity I think I know what the watsons were trying to portray now. There's actually no exploit but rather an intended design entailed. In the test provided in this report, it's only displaying the two different The thing is, between block 1 and 2, if you were the only depositor, you would be entitled to The sponsor already disputed #177. You may want to seek the sponsor's opinion on this report since it has not been reviewed by them. |
|
@mystery0x @WangSecurity Yeah, there is an oversight on our part regarding the impact. The inflated Alice is the first depositor depositing
There is no docs provided for this contest and readme also doesn't state any design choices. I believe this is more of a edge case.
Yes, but the first depositor is indeed getting an unfair advantage over other depositors.
If depositors want to get upset/discouraged over fair distribution of points then that's on them. To conclude, the severity is not as we mentioned in the report but the first depositor can still get |
To clarify, how many points would other depositors get in that scenario? |
@WangSecurity Points calculation depends on a lot of factors like total amount deposited, blockMultiplier, pointsPerBlock, etc. In the updated PoC I provided, all other values are default and total amount deposited is 5 ether(1 ether per person) + first deposit amount. There are 5 depositors including Alice. Alice gets
For example in the above example if we have a sixth depositor and the total deposited is 6 ether(1 ether per person) + first deposited amount. Alice will get |
|
Thank you for that detailed explanation. With that, I agree that it's indeed a bug and allows the first depositor to get significantly more points than other depositors with only 1 wei. Planning to accept the escalation and validate this issue with high severity. As I know the duplicate is #177, are there any additional duplicates? |
|
Each block the pool from the example receives 25e18 / 3 rewards, and distributes them between its stakers, proportionally to their stake. If by the end of the first block Alice was the only staker of the pool, why should she not receive |
@WangSecurity I respectfully disagree with this.
Since there are no other depositors in the first block besides Alice, she is entitled to the entire amount ( I believe this is the intended behavior. For example, if the protocol decided to distribute some tokens to depositors from block 1-10, the protocol calculates tokens distributed for each block using the points system. And this point is added to the each depositers points who has active deposits in that block. The basic idea is the protocol allocating a portion of the tokens to each block for distribution. Here is a document shared by the sponsor for explaining the point calculation in the Discord channel: Staking Algorithm by RareSkills. This will explain the calculations in more detail. I agree that this document is not mentioned anywhere in the README. |
#82, #111, and #172 should be duplicates too. However, I think the severity of this finding is low/informational and at best medium. The sponsor already disputed #177 and #172, signifying they're aware of it and wouldn't care much about it. The extra points (similarly of 18 decimals and linearly distributed too) gained isn't much (which is fully deserving for your early support); and, the first depositor depositing 1 wei will have to bet on no one is depositing for at least the first 12 seconds. |
|
There are no more duplicates that I can find.
I respectfully disagree with this. First 12 seconds do not count as early support. Also the extra points i.e |
|
Fully agree with the views of both @aslanbekaibimov and @0xAadi, this is completely normal behavior. If at any moment there is only one depositor, whether he deposit 1 wei or 100 eth, he should be able to get all the points generated by the blocks during that period, until someone else deposits and competes with them. This is entirely reasonable. |
|
@0xSandyy could you please share your counterarguments to this and this comments by @aslanbekaibimov @0xAadi? As I understand what you mean, for example, there are 10 points in each block, so if Alice is the only one who deposits in the first block (regardless of the amount) she gets all the points for that block? But what if one honest depositor is the only one in the first block and they deposit 1 ETH, they get 10 points. Then Alice deposits 1 wei in the second block and also gets 10 points? And in the new POC, if there are only two depositors in 2 blocks (1 depositor for each block), there would be no 8.333333333×10¹⁸ difference? |
Correct
If there is a second depositor, then total of their deposits are used for calculating the point share for each. Therefore, it will not be a 50:50 split, and the depositor with fewer deposits will receive a smaller point share.
In this situation, the honest depositor will receive the majority of the point share because the first depositor remains active in the second block alongside Alice, who has a significant deposit. Please see the test result below, which mocks the above scenario: Notice that the honest depositor, Bob, received most of the point share ( |
@WangSecurity They are right. Since Alice is the only depositor in the first block, she gets all the points. I am not denying that. Let me simplify again what is the issue I am trying to portray:
This was to prove point 4, thus Alice deposits The 4 is the same thing I was trying to prove here:
|
|
To clarify:
You made a typo and it should be "... who has an insignificant deposit", correct? @0xAadi Also can you please share the POC, it's the one used above, but with Bob depositing first? And the last question, if Alice deposits 1 wei in the first block, then Bob deposits 10 ETH in the second block. I assume he should have the majority of the points, not Alice? But as I see from the @0xSandyy POC, Alice remains that big part of the pot, or am I missing something? And @0xSandyy thank you for the response, don't think I have any questions about that specifically, only the ones above. |
|
Hi, @WangSecurity I updated my last comment as this was the attack flow in my mind the whole time and I feel the escalation is going all over the place now.
Also the updated comment will answer this too. |
@WangSecurity , In this scenario, it's evident that Alice receives all the points from the first block, let's say 10000 points. Then, in the second block, due to Bob's entry into competition with a larger deposit, he will receive most of the points from the second block, for example, the ratio of Bob to Alice being 9999:1. At the end of the second block, Alice still has more points than Bob, with a ratio of 10001:9999, because Bob only received a portion of the points from the second block, while Alice already claimed all the points from the first block. It's only from the third block onwards that Bob's score will surpass Alice's. My understanding of what @0xSandyy is saying is this: in any situation where there are no current deposits, anyone can use a very small amount to get all the points from a block. Am I understanding this correctly? But is this an issue? The process of get points inherently involves competition. If no one else is competing with me, wouldn't it be reasonable for me to use just 1 wei to get all the points? Anyone can attempt to deposit a very small amount when there are no other deposits, which essentially is a process of competition and game, isn't it? So, I fail to see why this is considered a valid issue. |
Yes, what I mean is the honest depositor will receive the majority of the point share because they have a large deposit.
function test_poc() public {
uint256 depositOfAlice = 1001 wei;
// 1001 wei is the limit set in MockStETH.sol:submit().
uint256 depositOfBob = 1 ether;
address alice = address(10);
vm.deal(alice, depositOfAlice);
address bob = address(20);
vm.deal(bob, depositOfBob);
uint256 poolId = sophonFarming.typeToId(
SophonFarmingState.PredefinedPool.wstETH
);
console.log("----------------------------------------------------------------");
console.log("Bob Deposits 1 ETH in Block",block.number);
vm.startPrank(bob);
sophonFarming.depositEth{value: depositOfBob}(
0,
SophonFarmingState.PredefinedPool.wstETH
);
vm.stopPrank();
vm.roll(block.number + 1);
console.log("--------------------ROLL TO BLOCK",block.number,"----------------------------");
console.log("Bob entitled to get :", sophonFarming.pendingPoints(poolId, bob));
console.log("----------------------------------------------------------------");
console.log("Alice Deposits 1001 wei in Block",block.number);
vm.startPrank(alice);
sophonFarming.depositEth{value: depositOfAlice}(
0,
SophonFarmingState.PredefinedPool.wstETH
);
vm.stopPrank();
vm.roll(block.number + 1);
console.log("--------------------ROLL TO BLOCK",block.number,"----------------------------");
console.log("Alice entitled to get:", sophonFarming.pendingPoints(poolId, alice));
console.log("Bob entitled to get :", sophonFarming.pendingPoints(poolId, bob));
console.log("----------------------------------------------------------------");
} |
@WangSecurity Please see the below POC function test_poc() public {
uint256 depositOfAlice = 1001 wei;
// 1001 wei is the limit set in MockStETH.sol:submit().
uint256 depositOfBob = 10 ether;
address alice = address(10);
vm.deal(alice, depositOfAlice);
address bob = address(20);
vm.deal(bob, depositOfBob);
uint256 poolId = sophonFarming.typeToId(
SophonFarmingState.PredefinedPool.wstETH
);
console.log("----------------------------------------------------------------");
console.log("Alice Deposits 1001 wei in Block",block.number);
vm.startPrank(alice);
sophonFarming.depositEth{value: depositOfAlice}(
0,
SophonFarmingState.PredefinedPool.wstETH
);
vm.stopPrank();
vm.roll(block.number + 1);
console.log("Total Points of Alice:", sophonFarming.pendingPoints(poolId, alice),"Points");
console.log("Total Points of Bob :", sophonFarming.pendingPoints(poolId, bob),"Points");
console.log("--------------------ROLL TO BLOCK",block.number,"----------------------------");
console.log("Bob Deposits 10 ETH in Block",block.number);
vm.startPrank(bob);
sophonFarming.depositEth{value: depositOfBob}(
0,
SophonFarmingState.PredefinedPool.wstETH
);
vm.stopPrank();
vm.roll(block.number + 1);
console.log("Total Points of Alice:", sophonFarming.pendingPoints(poolId, alice),"Points");
console.log("Total Points of Bob :", sophonFarming.pendingPoints(poolId, bob),"Points");
console.log("--------------------ROLL TO BLOCK",block.number,"----------------------------");
vm.roll(block.number + 1);
console.log("Total Points of Alice:", sophonFarming.pendingPoints(poolId, alice),"Points");
console.log("Total Points of Bob :", sophonFarming.pendingPoints(poolId, bob),"Points");
console.log("----------------------------------------------------------------");
}Here we can see that Alice is accumulating an insignificant amount of shares from Block 2 onwards, while Bob is accumulating the major portion of the points.
Also note that the protocol allows users to deposit right after creating the pool, but the point calculation only starts from the start block. This gives all users an equal opportunity to start depositing in any pool before the start block. |
|
Thanks to all the Watsons for these comments and for explaining it. I agree that it's a design of the points system and it works as it should be. The problem is that as the first depositor and the only one in the block, you indeed receive all the points, but with all the depositors after it, you will receive all the points as per your allocation inside the pool. It's not a security issue but rather a system to reward the first depositors, regardless of their deposit (be that 10000 ETH or 1 wei, you will receive all the points if you're the only depositor in the first block). Hence, planning to reject the escalation and leave the issue as it is. |
|
Result: |
sherlock-admin3
added
the
Escalation Resolved
|
Escalations have been resolved successfully! Escalation status:
|
dimulski
high
A malicious actor can manipulate the distribution of points
Summary
The
SophoFarming.solcontract main purpose is to calculate users points based on their deposits, it is stated that it will support different pools, where users can deposit the corresponding asset and gain points. New pools can be added after the contract has been initialized by the owner via the add() function. A malicious actor can manipulate the points for a certain pool, by backrunning the pool creation transaction and depositing 1 WEI, then in the next block he can fronrun all other transactions and call the updatePool() function, afterwards each user that deposits in that pool will be receiving much more points, than users that deposit in other pools.As we can see from the above code snippet
pointReward = blockMultiplier * _pointsPerBlock * _allocPoint / totalAllocPoint;Consider the following example:
If lpSupply is equal to 1 then accPointsPerShare will be 8333333333333333333333333333333333333 as 8333333333333333333333333333333333333 / 1 = 8333333333333333333333333333333333333 ≈ 8.3e37 .
However if the first deposit is 1e18 or more, the above calculations for accPointsPerShare will be as follows:
8333333333333333333333333333333333333 / 1e18 = 8333333333333333333 ≈ 8.3e18 which is a big difference.
Given the fact that the main purpose of the contract is to track points, I believe points manipulation is of high severity.
Vulnerability Detail
Gist
After following the steps in the above mentioned gist add the following test to the
AuditorTests.t.solfile:To run the test use:
forge test -vvv --mt test_ManipulatePointDistributionImpact
The distribution of points can be manipulated for a certain pool, and users who don't know about it will receive less rewards, for the same amount of deposits in different pools. The malicious actor is effectively stealing rewards from them. Additionally adding a new pool after some time has passed, and people have already deposited in other pools, if the attacker manipulates the point distribution in the new pool, all users who have already deposited in other pools up to that point in time will be in a big disadvantage.
Code Snippet
https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L411-L435
Tool used
Manual Review & Foundry
Recommendation
In the transaction that the contract is inititalized, or a new pool is added create a deposit with a 1e18 amount of the lpToken for the pool.
The text was updated successfully, but these errors were encountered: