#59007 [SC-Low] periodattimestamp returns current period instead of historical
Submitted on Nov 7th 2025 at 17:27:43 UTC by @Tomioka for Audit Comp | Firelight
- Report ID: #59007
- Report Type: Smart Contract
- Report severity: Low
- Target: https://github.com/firelight-protocol/firelight-core/blob/main/contracts/FirelightVault.sol
- Impacts:
- Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
The periodAtTimestamp() function is designed to return the period number for a given historical timestamp. However, due to the implementation of the private helper function _sinceEpoch(), it always uses Time.timestamp() (current block timestamp) instead of the provided timestamp parameter. This causes periodAtTimestamp() to always return the current period number regardless of what historical timestamp is passed to it.
Vulnerability Details
The Core Flaw:
The periodAtTimestamp() function at line 246-250 calls _sinceEpoch() to calculate elapsed time:
// Line 246-250: periodAtTimestamp function
function periodAtTimestamp(uint48 timestamp) public view returns (uint256) {
PeriodConfiguration memory periodConfiguration = periodConfigurationAtTimestamp(timestamp);
// solhint-disable-next-line max-line-length
return periodConfiguration.startingPeriod + _sinceEpoch(periodConfiguration.epoch) / periodConfiguration.duration;
}
The _sinceEpoch() helper function is defined at line 795-797:
// Line 795-797: _sinceEpoch implementation
function _sinceEpoch(uint48 epoch) private view returns (uint48) {
return Time.timestamp() - epoch;
}
Why This Is Wrong:
The periodAtTimestamp(uint48 timestamp) function receives a timestamp parameter that should be used to calculate the period number at that point in time. However:
- Line 247 correctly retrieves the
periodConfigurationfor the provided timestamp - Line 249 calls
_sinceEpoch(periodConfiguration.epoch)which calculatesTime.timestamp() - epoch Time.timestamp()returns the current block timestamp, not thetimestampparameter passed to the function- The result is always calculated based on the current time, making the input parameter effectively ignored
The Correct Implementation Should Be:
function periodAtTimestamp(uint48 timestamp) public view returns (uint256) {
PeriodConfiguration memory periodConfiguration = periodConfigurationAtTimestamp(timestamp);
// FIX: Use the provided timestamp parameter, not current time
return periodConfiguration.startingPeriod + (timestamp - periodConfiguration.epoch) / periodConfiguration.duration;
}
Root Cause Analysis:
The bug stems from the design of _sinceEpoch() as a helper function that always uses the current timestamp. This pattern works correctly for functions like currentPeriod(), currentPeriodStart(), and currentPeriodEnd() which are meant to operate on the current time. However, it breaks the contract when used in periodAtTimestamp(), which needs to calculate periods for arbitrary historical (or future) timestamps.
Functions Affected:
The following functions use _sinceEpoch():
periodAtTimestamp(uint48 timestamp): BROKEN, Should use provided timestampcurrentPeriodStart(): Correct: Intended to use current timecurrentPeriodEnd(): Correct: Intended to use current time
Impact Details
Primary Impact: Historical Query Failure
The primary consequence is that any attempt to query the period number for a historical timestamp will fail to return accurate historical data. Instead, it will always return the current period number.
Secondary Impact: Dependent System Failures
Any off chain systems, integrations, or internal logic that relies on periodAtTimestamp() to:
- Look up historical period numbers
- Validate timestamps against period boundaries
- Reconstruct historical state
- Perform period based calculations for past events
will receive incorrect data, leading to potential cascading failures.
Tertiary Impact: User Confusion
Users or integrators who call periodAtTimestamp() with a historical timestamp will receive confusing results:
- The function signature promises to return "the period number for the timestamp given"
- The return value will always be the current period
- No revert or error occurs, making the bug silent and hard to detect
Practical Example:
// Period 0: Jan 1 - Jan 7 (epoch = Jan 1)
// Period 1: Jan 8 - Jan 14
// Period 2: Jan 15 - Jan 21 (current period)
uint48 jan5 = 1704412800; // January 5, 2024 timestamp
// Expected: periodAtTimestamp(jan5) should return 0
// Actual: periodAtTimestamp(jan5) returns 2 (current period)
Affected Users:
- Off chain systems querying historical period data
- Analytics tools and dashboards
- Third party integrations that rely on period lookups
- Smart contracts that call this function externally
References
- Vulnerable code:
contracts/FirelightVault.sol:246-250 - Helper function:
contracts/FirelightVault.sol:795-797 - OpenZeppelin Time library: https://docs.openzeppelin.com/contracts/5.x/api/utils#Time
- NatSpec documentation at line 241-245 (describes intended behavior)
Proof of Concept
Proof of Concept
The complete runnable PoC : test/poc_period_bug.js.
Execution
# Install dependencies
npm install
# Run the PoC test
npx hardhat test test/poc_period_bug.js
Full PoC
const { expect } = require('chai')
const { ethers } = require('hardhat')
const mineTime = async (seconds) => {
await ethers.provider.send('evm_increaseTime', [seconds])
await ethers.provider.send('evm_mine', [])
}
describe('PoC: periodAtTimestamp Bug (Low)', () => {
async function deploySimpleVault() {
const ERC20Factory = await ethers.getContractFactory('MinimalERC20')
const token = await ERC20Factory.deploy('TestToken', 'TT', 6)
await token.waitForDeployment()
const VaultFactory = await ethers.getContractFactory('FirelightVault')
const [deployer, limitUpdater, blocklister, pauser, periodUpdater] = await ethers.getSigners()
const abiCoder = ethers.AbiCoder.defaultAbiCoder()
const initParams = abiCoder.encode([
'address','address','address','address','address','uint256','uint48'
],[
deployer.address,
limitUpdater.address,
blocklister.address,
pauser.address,
periodUpdater.address,
1_000_000_000n,
7 * 24 * 60 * 60
])
const vault = await ethers.upgrades.deployProxy(VaultFactory,[await token.getAddress(),'Share','Share',initParams])
return { vault }
}
it('returns current period instead of historical for past timestamp', async () => {
const { vault } = await deploySimpleVault()
// Capture an early timestamp
const startTs = (await ethers.provider.getBlock('latest')).timestamp
// Advance time by almost two periods (default period 1 week)
await mineTime(10 * 24 * 60 * 60) // 10 days
const nowTs = (await ethers.provider.getBlock('latest')).timestamp
const currentPeriodByView = await vault.currentPeriod()
const supposedPastPeriod = await vault.periodAtTimestamp(startTs)
// Expectation (correct logic): supposedPastPeriod should be lower than currentPeriodByView.
// Actual behavior (bug): function uses current timestamp internally and returns currentPeriodByView.
console.log('startTs:', startTs)
console.log('nowTs:', nowTs)
console.log('currentPeriod:', currentPeriodByView.toString())
console.log('periodAtTimestamp(startTs):', supposedPastPeriod.toString())
expect(supposedPastPeriod).to.equal(currentPeriodByView) // Showing the incorrect equality evidencing the bug.
})
})
Key Assertions
// Capture early timestamp
const startTs = (await ethers.provider.getBlock('latest')).timestamp
// Advance time by 10 days
await mineTime(10 * 24 * 60 * 60)
// Get current state
const currentPeriodByView = await vault.currentPeriod()
const supposedPastPeriod = await vault.periodAtTimestamp(startTs)
// BUG: Historical query returns current period
expect(supposedPastPeriod).to.equal(currentPeriodByView)