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

Protection Bypass in Parameter Updates

aariiif

Summary

BuyerAgent contract's parameter update functions (setFeeRoyalty and setAmountPerRound) can be executed in incorrect phases due to non-atomic phase validation, allowing manipulation of critical financial parameters during active trading periods.

Vulnerability Details

The phase validation in parameter update functions creates a race condition window between phase check and execution. The vulnerable code paths are: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/swan/BuyerAgent.sol#L380-L398

function setFeeRoyalty(uint96 _fee) public onlyOwner {
// @Issue - Phase check not atomic with parameter update
_checkRoundPhase(Phase.Withdraw);
if (_fee < 1 || _fee > 100) {
revert InvalidFee(_fee);
}
royaltyFee = _fee;
}
​
function setAmountPerRound(uint256 _amountPerRound) external onlyOwner {
// @Issue - Phase check not atomic with parameter update
_checkRoundPhase(Phase.Withdraw);
amountPerRound = _amountPerRound;
}

The contract attempts to enforce phase protection through _checkRoundPhase(Phase.Withdraw).

POC

// 1. Start in Withdraw phase
// 2. Call setFeeRoyalty or setAmountPerRound
// 3. Between phase check and execution, phase transitions
// 4. Parameter update executes in wrong phase
async function demonstrateRaceCondition() {
const buyerAgent = await BuyerAgent.deploy();
// Wait for Withdraw phase
await timeTravel(to_withdraw_phase);
// Start transaction but don't mine
const tx = buyerAgent.setFeeRoyalty(50);
// Force phase change
await timeTravel(phase_duration);
// Mine transaction - executes in wrong phase
await tx.wait();
}

Impact

  • Parameters could be modified during active trading phases

  • This could disrupt ongoing purchases and financial calculations

  • Breaks the phase isolation principle of the protocol

Recommendations

+ event ParameterUpdate(string indexed parameter, uint256 oldValue, uint256 newValue);
​
+ modifier enforceWithdrawPhase() {
+ (uint256 round, Phase currentPhase,) = getRoundPhase();
+ require(currentPhase == Phase.Withdraw, "Not in withdraw phase");
+ _;
+ }
​
- function setFeeRoyalty(uint96 _fee) public onlyOwner {
+ function setFeeRoyalty(uint96 _fee) public onlyOwner enforceWithdrawPhase {
- _checkRoundPhase(Phase.Withdraw);
if (_fee < 1 || _fee > 100) {
revert InvalidFee(_fee);
}
+ uint96 oldFee = royaltyFee;
royaltyFee = _fee;
+ emit ParameterUpdate("royaltyFee", oldFee, _fee);
}
​
- function setAmountPerRound(uint256 _amountPerRound) external onlyOwner {
+ function setAmountPerRound(uint256 _amountPerRound) external onlyOwner enforceWithdrawPhase {
- _checkRoundPhase(Phase.Withdraw);
+ uint256 oldAmount = amountPerRound;
amountPerRound = _amountPerRound;
+ emit ParameterUpdate("amountPerRound", oldAmount, _amountPerRound);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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