Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Phase calculation inaccuracy will always extend sell phase and cut withdrawal phase time

jsondoge

Summary

Due to incorrect if comparison sell period will always be extended by 1 unit and cut withdrawal time by the same amount.

Vulnerability Details

In the phase comparison:

if (roundTime <= params.sellInterval) {
return (round, Phase.Sell, params.sellInterval - roundTime);
} else if (roundTime <= params.sellInterval + params.buyInterval) {
return (round, Phase.Buy, params.sellInterval + params.buyInterval - roundTime);
} else {
return (round, Phase.Withdraw, cycleTime - roundTime);
}

three ifs determine which phase it is using the current roundTime (which is a timestamp remainder). But because incorrect comparison sign "<=" is used it always extends the first phase interval (sellInterval). Look at "Tools Used" for an example case.

This also returns incorrect "Time till next phase", last of the three parameters. As both Sell and Buy phases, can reach 0.

Impact

Incorrect phase calculation only shifts 1 second from last to first phase. But because Base blocks at the time of writing are minted every 2 seconds, the error will be more significant because (block.timestamp updated frequently - higher chance to fall in the shifted seconds):

  • Transactions that are meant for "Buy" phase - can fall in the extended "Sell" phase and fail.

  • If "Withdrawal" phase is set to 1 seconds - it will be skipped (never reached).

  • Even if "Sell" interval is set to 0 seconds and should skip, it will still be reached.

The comparison is at the core of the protocol and often used, can cause failing transactions and invalid protocol functionality (skipping "Withdrawal"). But no loss of funds (apart from gas) - Medium.

Tools Used

Manual review + hardhat tests

const phaseIndexToStr = ["Buy", "Sell", "Withdraw"];
const getBlockTimestamp = async () => {
const latestBlock = await ethers.provider.getBlock("latest");
if (!latestBlock) {
throw new Error("No latest block found");
}
return latestBlock.timestamp;
};
​
describe("Phases calculation", function () {
it("Calculation extends sell and cuts withdraw phase short", async function () {
// all phases set to equal = 2
await swan
.connect(dria)
.setMarketParameters({ ...MARKET_PARAMETERS, buyInterval: 2, sellInterval: 2, withdrawInterval: 2 });
​
// after new market params are set the round and phase restarts
for (let i = 0; i < 7; i++) {
const phaseRes = await buyerAgent.getRoundPhase();
const markSetOn = Number((await swan.getCurrentMarketParameters()).timestamp);
const currentTime = await getBlockTimestamp();
console.log("Time since market set:", currentTime - markSetOn);
console.log("Current phase:", phaseIndexToStr[Number(phaseRes[1])]);
console.log("Time till next:", Number(phaseRes[2]));
await ethers.provider.send("evm_increaseTime", [1]);
await ethers.provider.send("evm_mine");
}
});
});

The following test prints out:

Time since market set: 0
Current phase: Sell
Time till next: 2
Time since market set: 1
Current phase: Sell
Time till next: 1
Time since market set: 2
Current phase: Sell // <- Sell phases lasts for 3 seconds
Time till next: 0 // <- shouldn't reach zero
Time since market set: 3
Current phase: Buy
Time till next: 1
Time since market set: 4
Current phase: Buy
Time till next: 0 // <- shouldn't reach zero
Time since market set: 5
Current phase: Withdraw
Time till next: 1 // <- Withdraw phases lasts for 1 second
Time since market set: 6
Current phase: Sell
Time till next: 2

As we can see even though all phases should take the same amount of time - Sell is always extended buy 1 and Withdraw phase cut short buy one.

Recommendations

Replace comparison from "<=" to "<" in both sellInterval and buyInterval.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Phase timing in `_computePhase`

Appeal created

ljj Auditor
8 months ago
ljj Auditor
8 months ago
newspacexyz Auditor
8 months ago
nicholeconn Auditor
8 months ago
ljj Auditor
8 months ago
0xw3 Auditor
8 months ago
pyro Auditor
8 months ago
jsondoge Submitter
8 months ago
inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Phase timing in `_computePhase`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.