Quill Findings: APY Miscalculation in Staking Rewards
Learn how a missing lock-duration factor in staking reward math paid the full 8% yearly APY on every 60-day lock, turning intended yield into roughly 49%.
This is the third blog in our series Quill Findings, where we share vulnerabilities our auditors found during client audits, for auditor and developer education. If you missed the earlier ones, you can read the first one here and the second one here.
Today we have a staking protocol, where users lock their tokens for 60 days and earn 8% APY on the staked amount. The reward math missed calculated rewards, and turned an 8% yearly yield into roughly 49%.
How the reward calculation works in general
APY is a yearly rate and a 60 days lock period should only give rewards generated by the 60days part of that 8% yearly calculation, not all of it. In code below you can see that how 60 days rewards calculation should work:
1// 8% per year, in basis points
2uint256 constant APY_BPS = 800;
3uint256 constant LOCK_PERIOD = 60 days;
4
5// reward = amount x yearly rate x (lock time / 1 year)
6//
7// 10,000 tokens x 8% x (60 / 365)
8// = 10,000 x 0.08 x 0.164
9// = ~131 tokens
10Here a user is staking 10,000 tokens for one lock period(60 days) and should get back about 10,131 tokens.
What was wrong in the contract
The contract calculated the rewards like this:
1function calculateReward(uint256 amount) public pure returns (uint256) {
2 // bug: pays the full yearly 8% on every 60-day lock,
3 // the lock duration never enters the math
4 //
5 // 10,000 tokens -> 800 tokens reward
6 // expected -> ~131 tokens
7 // overpaid -> ~6x on every lock
8 return amount * APY_BPS / 10_000;
9}
10The yearly rate is treated as a 60 days rate and every lock pays the full 8%, whether the tokens were locked for 60 days or a whole year.
A year has over six 60days periods cycles, so a user who keeps locking again and again earns far more than 8%:
1// intended: 8% a year
2// actual, re-locking(original deposit only): 8% x 6.08 periods = ~49% a year
3// actual, restaking(compunding: deposit + previous reward): (1.08)^6.08 - 1 = ~60% a year
4Understand the Vulnerability Flow
Impact
No special access or attacker contract was required. Anyone who stakes normally gets overpaid. This was a business logic implementation bug.
The rewards come from a pool that team funds. That pool is create for 8% a year, but it pays out as if the rate were about six times higher and tt drains much faster than planned, and as TVL grows.
Proof of concept
This is a minimal example to show the bug from the code side. Its a replica of vulnerable code
src/MockToken.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
5
6contract MockToken is ERC20 {
7 constructor() ERC20("Mock Token", "MOCK") {}
8
9 function mint(address to, uint256 amount) external {
10 _mint(to, amount);
11 }
12}
13src/StakingVulnerable.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
5
6contract StakingVulnerable {
7 IERC20 public immutable token;
8
9 uint256 public constant APY_BPS = 800; // 8% a year
10 uint256 public constant LOCK_PERIOD = 60 days;
11
12 struct Stake {
13 uint256 amount;
14 uint256 unlockAt;
15 }
16
17 mapping(address => Stake) public stakes;
18
19 constructor(IERC20 _token) {
20 token = _token;
21 }
22
23 function stake(uint256 amount) external {
24 require(stakes[msg.sender].amount == 0, "already staked");
25 token.transferFrom(msg.sender, address(this), amount);
26 stakes[msg.sender] = Stake(amount, block.timestamp + LOCK_PERIOD);
27 }
28
29 // bug: the full yearly rate is paid for a 60-day lock
30 function calculateReward(uint256 amount) public pure returns (uint256) {
31 return amount * APY_BPS / 10_000;
32 }
33
34 function withdraw() external {
35 Stake memory s = stakes[msg.sender];
36 require(s.amount > 0, "no stake");
37 require(block.timestamp >= s.unlockAt, "still locked");
38 delete stakes[msg.sender];
39 token.transfer(msg.sender, s.amount + calculateReward(s.amount));
40 }
41}
42Create the fixed version. It's the same contract with only calculateReward changed:
1sed -e 's/contract StakingVulnerable/contract StakingFixed/' \
2 -e 's|return amount \* APY_BPS / 10_000;|return amount * APY_BPS * LOCK_PERIOD / (10_000 * 365 days);|' \
3 src/StakingVulnerable.sol > src/StakingFixed.sol
4test/APYOverpay.t.sol:
1// SPDX-License-Identifier: MIT
2pragma solidity ^0.8.24;
3
4import {Test, console2} from "forge-std/Test.sol";
5import {MockToken} from "../src/MockToken.sol";
6import {StakingVulnerable} from "../src/StakingVulnerable.sol";
7import {StakingFixed} from "../src/StakingFixed.sol";
8
9contract APYOverpayTest is Test {
10 MockToken token;
11 address alice = makeAddr("alice");
12 address bob = makeAddr("bob");
13
14 uint256 constant STAKE = 10_000e18;
15 uint256 constant POOL = 800e18; // reward pool funded by the team
16
17 function setUp() public {
18 token = new MockToken();
19 token.mint(alice, STAKE);
20 token.mint(bob, STAKE);
21 }
22
23 function _stake(address user, address staking) internal {
24 vm.startPrank(user);
25 token.approve(staking, STAKE);
26 StakingVulnerable(staking).stake(STAKE);
27 vm.stopPrank();
28 }
29
30 function test_RewardIsFullYearlyRate() public {
31 StakingVulnerable staking = new StakingVulnerable(token);
32
33 uint256 paid = staking.calculateReward(STAKE);
34 uint256 expected = STAKE * 800 * 60 days / (10_000 * 365 days);
35
36 console2.log("reward paid for 60 days :", paid / 1e18);
37 console2.log("reward expected (8% APY):", expected / 1e18);
38
39 assertEq(paid, 800e18, "full 8% paid on a 60-day lock");
40 assertGt(paid, expected * 6, "more than 6x the intended reward");
41 }
42
43 function test_FirstStakerDrainsPool() public {
44 StakingVulnerable staking = new StakingVulnerable(token);
45 token.mint(address(staking), POOL);
46
47 _stake(alice, address(staking));
48 _stake(bob, address(staking));
49 vm.warp(block.timestamp + 60 days);
50
51 vm.prank(alice);
52 staking.withdraw();
53 console2.log("alice received :", token.balanceOf(alice) / 1e18);
54
55 // the pool is already gone, bob can't even get his principal back
56 vm.prank(bob);
57 vm.expectRevert();
58 staking.withdraw();
59 console2.log("bob withdraw : reverted");
60 }
61
62 function test_FixedPaysOnlySixtyDays() public {
63 StakingFixed staking = new StakingFixed(token);
64 token.mint(address(staking), POOL);
65
66 _stake(alice, address(staking));
67 _stake(bob, address(staking));
68 vm.warp(block.timestamp + 60 days);
69
70 vm.prank(alice);
71 staking.withdraw();
72 vm.prank(bob);
73 staking.withdraw();
74
75 console2.log("alice received :", token.balanceOf(alice) / 1e18);
76 console2.log("bob received :", token.balanceOf(bob) / 1e18);
77 console2.log("pool left :", token.balanceOf(address(staking)) / 1e18);
78
79 assertEq(token.balanceOf(alice), STAKE + staking.calculateReward(STAKE));
80 assertEq(token.balanceOf(bob), STAKE + staking.calculateReward(STAKE));
81 }
82}
83Run it:
1forge test --match-contract APYOverpayTest -vv
2Output:
1Ran 3 tests for test/APYOverpay.t.sol:APYOverpayTest
2[PASS] test_FirstStakerDrainsPool() (gas: 440263)
3Logs:
4 alice received : 10800
5 bob withdraw : reverted
6
7[PASS] test_FixedPaysOnlySixtyDays() (gas: 503675)
8Logs:
9 alice received : 10131
10 bob received : 10131
11 pool left : 536
12
13[PASS] test_RewardIsFullYearlyRate() (gas: 16956)
14Logs:
15 reward paid for 60 days : 800
16 reward expected (8% APY): 131
17
18Suite result: ok. 3 passed; 0 failed; 0 skipped
19Here's what the three tests show.
test_RewardIsFullYearlyRate: for 10,000 tokens, the contract pays 800 tokens for a 60-day lock, instead of 131.
test_FirstStakerDrainsPool: the team funds the pool with 800 tokens, which is more than enough for two users at the real reward distribution. Alice and Bob each stake 10,000 and after 60 days, Alice withdraws 10,800 and takes the whole pool. When Bob tries to withdraw, the contract owes him 10,800 but only holds 10,000, so his withdrawal reverts. Bob can't even get his principal back.
test_FixedPaysOnlySixtyDays: the same flow on the fixed contract. Both users get about 10,131 back, and 536 tokens are still left in the pool.
The fix
Scale the yearly rate down to the lock duration.
1- return amount * APY_BPS / 10_000;
2+ return amount * APY_BPS * LOCK_PERIOD / (10_000 * 365 days);
31function calculateReward(uint256 amount) public pure returns (uint256) {
2 // fix: only pay the 60-day share of the yearly rate
3 // multiply first, divide once at the end,
4 // so integer division doesn't round rewards away
5 //
6 // 10,000 tokens -> ~131 tokens, matches 8% APY
7 return amount * APY_BPS * LOCK_PERIOD / (10_000 * 365 days);
8}
9The order of operations matters here. If you divide first, for example LOCK_PERIOD / 365 days, Solidity's integer division rounds it down to 0, and every user gets nothing. Multiplying everything first and dividing once at the end keeps the result accurate.
If the protocol ever supports more than one lock period, pass the actual lock duration into the function instead of relying on a single constant. With this, the same formula stays correct for every option.
Conclusion
Nothing in this bug reverts, and no test fails unless it checks the reward amount itself. The contract runs smoothly while paying far more than intended. When a rate is yearly, every payout needs to know how long the money was locked, and here, one missing time factor turned 8% into 49%.
Contents
