hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

Smart contracts for the Metrom project.
GNU General Public License v3.0
0 stars 0 forks source link

Off-by-one timestamp error #24

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x8b8b1f5a83a884203e202d43e55b44f7399a774dd193897c6e98a017b51f251e Severity: low

Description: Description

In Solidity, using >= or <= to compare against block.timestamp (alias now) may introduce off-by-one errors due to the fact that block.timestamp is only updated once per block and its value remains constant throughout the block's execution. If an operation happens at the exact second when block.timestamp changes, it could result in unexpected behavior. To avoid this, it's safer to use strict inequality operators (> or <). For instance, if a condition should only be met after a certain time, use block.timestamp > time rather than block.timestamp >= time. This way, potential off-by-one errors due to the exact timing of block mining are mitigated, leading to safer, more predictable contract behavior.

Attachments

  1. Proof of Concept (PoC) File

['160']

160:     function createCampaigns(CreateBundle[] calldata _bundles) external { // <= FOUND
161:         uint32 _fee = _resolvedFee();
162:         uint32 _minimumCampaignDuration = minimumCampaignDuration;
163:         uint32 _maximumCampaignDuration = maximumCampaignDuration;
164: 
165:         for (uint256 _i = 0; _i < _bundles.length; _i++) {
166:             CreateBundle memory _bundle = _bundles[_i];
167: 
168:             if (_bundle.pool == address(0)) revert InvalidPool();
169:             if (_bundle.from <= block.timestamp) revert InvalidFrom(); // <= FOUND
170:             if (
171:                 _bundle.to < _bundle.from + _minimumCampaignDuration
172:                     || _bundle.to - _bundle.from > _maximumCampaignDuration
173:             ) revert InvalidTo();
174:             if (
175:                 _bundle.rewardTokens.length == 0 || _bundle.rewardTokens.length > MAX_REWARDS_PER_CAMPAIGN
176:                     || _bundle.rewardTokens.length != _bundle.rewardAmounts.length
177:             ) revert InvalidRewards();
178: 
179:             bytes32 _id = _campaignId(_bundle);
180:             Campaign storage campaign = campaigns[_id];
181:             if (campaign.from != 0) revert CampaignAlreadyExists();
182: 
183:             campaign.owner = msg.sender;
184:             campaign.chainId = _bundle.chainId;
185:             campaign.pool = _bundle.pool;
186:             campaign.from = _bundle.from;
187:             campaign.to = _bundle.to;
188:             campaign.specification = _bundle.specification;
189:             campaign.rewards = _bundle.rewardTokens;
190: 
191:             uint256[] memory _feeAmounts = new uint256[](_bundle.rewardTokens.length);
192:             uint256[] memory _rewardAmountsMinusFees = new uint256[](_bundle.rewardTokens.length);
193:             for (uint256 _j = 0; _j < _bundle.rewardTokens.length; _j++) {
194:                 address _token = _bundle.rewardTokens[_j];
195:                 if (_token == address(0)) revert InvalidRewards();
196: 
197:                 uint256 _amount = _bundle.rewardAmounts[_j];
198:                 if (_amount == 0) revert InvalidRewards();
199: 
200:                 for (uint256 _k = _j + 1; _k < _bundle.rewardTokens.length; _k++) {
201:                     if (_token == _bundle.rewardTokens[_k]) revert InvalidRewards();
202:                 }
203: 
204:                 uint256 _feeAmount = _amount * _fee / UNIT;
205:                 uint256 _rewardAmountMinusFees = _amount - _feeAmount;
206:                 claimableFees[_token] += _feeAmount;
207: 
208:                 _feeAmounts[_j] = _feeAmount;
209:                 _rewardAmountsMinusFees[_j] = _rewardAmountMinusFees;
210: 
211:                 Reward storage reward = campaign.reward[_token];
212:                 reward.amount = _rewardAmountMinusFees;
213:                 reward.unclaimed = _rewardAmountMinusFees;
214: 
215:                 IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);
216:             }
217: 
218:             emit CreateCampaign(
219:                 _id,
220:                 msg.sender,
221:                 _bundle.chainId,
222:                 _bundle.pool,
223:                 _bundle.from,
224:                 _bundle.to,
225:                 _bundle.specification,
226:                 _bundle.rewardTokens,
227:                 _rewardAmountsMinusFees,
228:                 _feeAmounts
229:             );
230:         }
231:     }
luzzif commented 4 months ago

Could you please provide a PoC for this?