Storage Manual Review Findings

SEG-01M: Incorrect Assignment of Initial BLID Per Block

Type	Severity	Location
Logical Fault		Storage.sol:L141-L149

Description:

The setBoostingInfo function can be invoked an arbitrary number of times and on each time it will overwrite the previous initBlidPerBlock.

Impact:

As the initBlidPerBlock value is meant to be an external data point utilized by other contracts, it is impossible to assess what the impact of this exhibit is.

Example:

Copy
141function setBoostingInfo(uint256 _maxBlidperUSD, uint256 _blidperBlock) external onlyOwner {
142    _boostingUpdateAccBlidPerShare();
143
144    maxBlidPerUSD = _maxBlidperUSD;
145    blidPerBlock = _blidperBlock;
146    initBlidPerBlock = _blidperBlock;
147
148    emit SetBoostInfo(_maxBlidperUSD, _blidperBlock);
149}

Recommendation:

We advise the initBlidPerBlock value to be set only once, properly containing the value of the "initial" BLID per block.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The initBlidPerBlock variable has been omitted from the codebase instead, rendering this exhibit inapplicable.

SEG-02M: Incorrect Interface Usage

Type	Severity	Location
Logical Fault		Storage.sol:L233, L387, L406, L631

Description:

The referenced statements cast an EIP-20 token to the AggregatorV3Interface incorrectly.

Impact:

As the decimals function signatures between AggregatorV3Interface and IERC20Metadata match, there is no issue in the current usage of the interface in the code. Nevertheless, the proper interface should be imported and utilized to simultaneously increase the legibility of the codebase and decrease the likelihood of an error arising in the future.

Example:

Copy
233uint8 decimals = AggregatorV3Interface(token).decimals();

Recommendation:

We advise an interface specifically for EIP-20 tokens to be utilized, such as IERC20Metadata by OpenZeppelin, to ensure that incorrect functions are not invoked in any future version of the codebase.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

All referenced instances of improper interface usage have been remediated by utilizing the IERC20MetadataUpgradeable interface instead which the tokens are expected to conform to.

SEG-03M: Significant Centralization of System Configuration

Type	Severity	Location
Centralization Concern		Storage.sol:L123-L124, L130-L134, L141-L149, L196-L200

Description:

All crucial aspects of the Storage's staking system are in control of the owner of the contract as they can arbitrarily set the logic address, the token supported, the BLID token, and more.

Example:

Copy
116/**
117 * @notice Set blid in contract
118 * @param _blid address of BLID
119 */
120function setBLID(address _blid) external onlyOwner {
121    BLID = _blid;
122
123    emit SetBLID(_blid);
124}
125
126/**
127 * @notice Set blid in contract
128 * @param _boostingAddress address of expense
129 */
130function setBoostingAddress(address _boostingAddress) external onlyOwner {
131    boostingAddress = _boostingAddress;
132
133    emit SetBoostingAddress(boostingAddress);
134}
135
136/**
137 * @notice Set boosting parameters
138 * @param _maxBlidperUSD max value of BLID per USD
139 * @param _blidperBlock blid per Block
140 */
141function setBoostingInfo(uint256 _maxBlidperUSD, uint256 _blidperBlock) external onlyOwner {
142    _boostingUpdateAccBlidPerShare();
143
144    maxBlidPerUSD = _maxBlidperUSD;
145    blidPerBlock = _blidperBlock;
146    initBlidPerBlock = _blidperBlock;
147
148    emit SetBoostInfo(_maxBlidperUSD, _blidperBlock);
149}

Recommendation:

We advise this level of centralization to be re-assessed, certain variables to be made immutable (as the contract is upgradeable regardless), and the functions that need to be adjustable to be invoke-able solely by governance mechanisms rather than central entities.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The Bolide Finance team assessed this exhibit and opted to retain the current ownership structure in place as they wish to be able to adjust the sensitive variables of the protocol post-deployment. They expect to increase the decentralization of the protocol in future updates of the protocol.

SEG-04M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		Storage.sol:L69

Description:

The contract is meant to be upgradeable yet does not properly protect its logic deployment from malicious initializations.

Example:

Copy
69function initialize(address _logicContract) external initializer {

Recommendation:

We advise a constructor to be introduced that invokes the initializer modifier of the Initializable contract to prevent the base implementation from ever being initialized.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

A constructor has been adequately introduced initializing the base implementation of the contract.

SEG-05M: Incorrect Payable Definition of Deposit Functions

Type	Severity	Location
Language Specific		Storage.sol:L209, L223

Description:

The deposit and depositOnBehalf functions can be invoked via a transaction that also transmits ether, causing the funds to be permanently locked in the contract.

Impact:

Any native funds transmitted alongside a deposit / depositOnBehalf call will be permanently locked in the contract unless it is upgraded.

Example:

Copy
209function deposit(uint256 amount, address token) external payable isUsedToken(token) whenNotPaused {
210    depositInternal(amount, token, msg.sender);
211}
212
213/**
214 * @notice Deposit amount of token on behalf of depositor wallet
215 * @param amount amount of token
216 * @param token address of token
217 * @param accountAddress Address of depositor
218 */
219function depositOnBehalf(
220    uint256 amount,
221    address token,
222    address accountAddress
223) external payable isUsedToken(token) whenNotPaused {

Recommendation:

We advise both functions to be set as "non-payable" by omitting the payable specifier from their function declarations.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The deposit and depositOnBehalf functions no longer contain the payable attribute thus alleviating this exhibit.

SEG-06M: Incorrect Re-Calculation of Deposit Amount

Type	Severity	Location
Mathematical Operations		Storage.sol:L313

Description:

The depositBLID function attempts to accommodate for over-deposited BLID amounts in the depositAmount calculation, however, the calculation can underflow if the user had a significant amount of BLID actively deposited (i.e. blidDeposit) prior to invoking depositBLID and the assets they have deposited have sharply dropped in USD evaluation. This would cause the overAmount to exceed the amount being deposited and thus cause the deposit to incorrectly fail.

Impact:

This issue essentially prevents a user to "over-deposit" funds while their limit is not satisfied, however, this should be a desirable scenario as a user may anticipate their assets increasing in USD value and thus would want to deposit BLID units beforehand, automatically compounding them via limit re-adjustment via _claimBoostingRewardBLIDInternal.

Example:

Copy
296function depositBLID(uint256 amount) external whenNotPaused {
297    require(amount > 0, "E3");
298    uint256 usdDepositAmount = balanceOf(msg.sender);
299    require(usdDepositAmount > 0, "E11");
300
301    BoostInfo storage userBoost = userBoosts[msg.sender];
302
303    _claimBoostingRewardBLIDInternal(msg.sender, false);
304    IERC20Upgradeable(BLID).safeTransferFrom(msg.sender, address(this), amount);
305
306    // Adjust blidOverDeposit
307    uint256 totalAmount = userBoost.blidDeposit + amount;
308    uint256 blidDepositLimit = (usdDepositAmount * maxBlidPerUSD) / 1e18;
309    uint256 depositAmount = amount;
310    if (totalAmount > blidDepositLimit) {
311        uint256 overAmount = totalAmount - blidDepositLimit;
312        userBoost.blidOverDeposit += overAmount;
313        depositAmount = amount - overAmount;
314    }
315
316    userBoost.blidDeposit += depositAmount;
317    totalSupplyBLID += amount;
318
319    // Save rewardDebt
320    userBoost.rewardDebt = (userBoost.blidDeposit * accBlidPerShare) / 1e18;
321
322    emit DepositBLID(msg.sender, amount);
323}

Recommendation:

We advise the depositBLID function to properly recalculate the active BLID deposited by first performing the deposit and then re-calculating the limits.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

All BLID deposit adjustment mechanisms have been refactored to utilize the _boostingAdjustAmount function that was newly introduced to the codebase. Within it, the BLID limits are properly re-calculated regardless of what the blidOverDeposit amount is thus alleviating this exhibit in full.

SEG-07M: Inexistent Initialization of Reward Block

Type	Severity	Location
Logical Fault		Storage.sol:L764

Description:

The linear per-block reward emission schedule of the Bolide system is currently incorrect as it does not initialize the lastRewardBlock variable to the block the system was activated. As a result, the first _boostingUpdateAccBlidPerShare invocation will create an abnormally high accBlidPerShare due to treating the current block.number as the number of blocks passed.

Impact:

While the issue is not actively exploitable in its present state, the reward per share would start at an abnormally high value that would render it more prone to overflows occurring as the system evolves.

Example:

Copy
756/**
757 * @notice update Accumulated BLID per share
758 */
759function _boostingUpdateAccBlidPerShare() internal {
760    if (block.number <= lastRewardBlock) {
761        return;
762    }
763
764    uint256 passedblockcount = block.number - lastRewardBlock;
765    accBlidPerShare = accBlidPerShare + (passedblockcount * blidPerBlock);
766    lastRewardBlock = block.number;
767}

Recommendation:

We advise the initialize code of the contract to properly set lastRewardBlock to the block.number the contract was initialized in (or potentially a deployer-defined future one) to ensure rewards are emitted in a proper per-block schedule.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The setBoostingInfo function, a prerequisite for the _boostingUpdateAccBlidPerShare functioning properly, now initializes the lastRewardBlock to the current block.number if it represents zero. As a result, we consider this exhibit alleviated.

SEG-08M: Inexplicable Usage of Signed Integer Type

Type	Severity	Location
Mathematical Operations		Storage.sol:L243, L247, L253, L256-L257, L443, L449, L454, L639, L643, L645-L650, L657, L708, L710

Description:

The referenced calculations are performing type-casts from an unsigned integer type (i.e. uintX) to a signed integer type (i.e. intX) whilst the codebase itself is not meant to handle negative values / signed types.

Impact:

Casting overflows can presently occur in the codebase and are not covered by the built-in safe arithmetic engine of Solidity contrary to popular belief.

Example:

Copy
639depositor.tokenTime[token] += int256(block.timestamp * (amountExp18));

Recommendation:

We advise all types to be set to unsigned values as presently, the int256 etc. type-casts are performed unsafely. Alternatively, if the type-casts need to remain in the codebase they should be performed via the usage of safe cast libraries by evaluating that the value being cast is less-than-or-equal-to the maximum value representable by signed integers (i.e. type(int256).max) which is significantly less than the maximum of unsigned integers (i.e. type(uint256).max).

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

After discussion with the Bolide Finance team, we assessed that negative values are indeed potentially expected in the system's architecture. As such, the Bolide Finance team proceeded with wrapping all unsafe casting statements using the SafeCastUpgradeable library of OpenZeppelin, rendering all of them safe to use and alleviating this exhibit.

SEG-09M: Potentially Insecure Decimal Normalizations

Type	Severity	Location
Logical Fault		Storage.sol:L235, L388, L407, L631

Description:

The referenced decimal normalizations assume that the underlying token contains at most 18 decimal places, however, EIP-20 compliant tokens with more than 18 decimals do exist and would significantly misbehave in the accounting system of the contract.

Impact:

While the impact of the exhibit is significant, the likelihood of it occurring should be close to inexistent as non-18 decimal tokens are rarely expected to be supported in a staking system. In any case, the code should outright prohibit them rather than permitting them to be included in the system of supported tokens.

Example:

Copy
235uint256 amountExp18 = amount * 10**(18 - decimals);

Recommendation:

We advise the system to either mandate that all tokens accepted by it via addToken have less-than-or-equal-to 18 decimals or to properly accommodate for more than 18 decimals by performing a division instead of a multiplication as part of the normalization process.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The Bolide Finance team has applied the first of the two recommended course of actions, introducing a decimal check in the addToken function that addresses this exhibit.

SEG-10M: Inexistent Application of Chainlink Security Practices

Type	Severity	Location
Logical Fault		Storage.sol:L517, L540

Description:

As explained in their documentation, contracts should apply multiple risk mitigation measures as Chainlink oracles are not infallible.

Impact:

While this type of finding would usually be set to minor, Chainlink's recent historical LUNA price-feed pause which led to lending protocols failing causes the severity of this exhibit to be set to medium. All Chainlink integrations should make use of at least those two safeguards described in this exhibit.

Example:

Copy
517uint256(oracle.latestAnswer()) *

Recommendation:

Of the security practices described, we advise a manual kill-switch to be introduced as well as an extreme price deviation event to halt operation by retaining a history of the most recent price fluctuations and comparing the deviation-per-second detected. As a final note, we advise the latestRoundData function to be utilized over latestAnswer as the latter has been officially deprecated by Chainlink.

Alleviation (fcd1a542d2):

While a kill-switch was not introduced, the code was properly updated to invoke the latestRoundData of Chainlink over the latestAnswer getter function. We would like to note that this partially addresses the exhibit and that a mechanism to recover from a price reporting flaw should be introduced.

Alleviation (c2c03f7731):

A permittable oracle price deviation per second was introduced to the codebase, evaluating whether the deviation per second between two price measurements exceeds the oracleDeviationLimit within Storage::addEarn and reverting in such a case. This security measure is not in place within the Storage::balanceOf & Storage::getTotalDeposit contract functions, however, the Bolide Finance team stated that they consider these functions to be of inconsequential security concern. As no contracts integrating with Storage were in scope of the audit, we cannot assess the validity of this statement. To this end, we will consider this exhibit alleviated based on the assumption that the Bolide Finance team's statements are true.

SEG-11M: Re-Initialization of Upgraded Implementation

Type	Severity	Location
Language Specific		Storage.sol:L70-L71

Description:

The __Ownable_init and __Pausable_init functions are meant to be invoked the first time the contract is deployed, however, they are currently present in a new version of the protocol that will be deployed over an existing proxy pattern.

Impact:

As the ownership of the contract is presently in control of a multi-signature wallet rather than the deployer (presumably), the initialize hook will transfer ownership to the deployer which then may not properly transfer ownership back to the original owners again.

Example:

Copy
69function initialize(address _logicContract) external initializer {
70    OwnableUpgradeable.__Ownable_init();
71    PausableUpgradeable.__Pausable_init();
72    logicContract = _logicContract;
73}

Recommendation:

We advise the initialization of the base implementations to be omitted as currently, ownership will be transferred to the deployer without any validation checks and the contract will be unpaused immediately on initialization.

Alleviation (fcd1a542d2):

The Bolide Finance team stated that the deployment account (ProxyAdmin) is controlled by their multi-signature wallet. As a result, the current code will re-initialize the OwnableUpgradeable contract to the deploying address which is the same as the live version's owner. Given that no security implications arise from the deployment workflow that Bolide Finance described, we consider this exhibit acknowledged.

We would like to note that the optimal remediation is to indeed remove the initializations as they will presently be executed on the existing storage space of the live StorageV21 deployment.

Alleviation (c2c03f7731):

The Storage::initialize function will operate on the storage space of the proxy while executing the logic implementation's contract code. Currently, Bolide Finance has a proxy (contract A) that points to a Storage implementation of the past (contract B).

The proxy was initialized using the past implementation thus executing OwnableUpgradeable::__Ownable_init and PausableUpgradeable::__Pausable_init during its initialization. This affected the storage space of contract A.

If the Bolide Finance team deploys a new implementation that the proxy of contract A will point to, the Storage::initialize function will not be invoke-able. As such, its presence in the code is inefficient. As an added point, the oracleDeviationLimit assignment introduced will not be executed either thus requiring a manual Storage::setOracleDeviationLimit invocation to function as expected.

SEG-12M: Incorrect Time Subtractions

Type	Severity	Location
Logical Fault		Storage.sol:L243, L247, L253, L255-L257

Description:

The withdrawal mechanism of the Storage contract is incorrect in that it performs a "subtraction" of token deposit time which has not been accounted for yet. As an example, a deposit transaction at timestamp X and a withdrawal transaction at timestamp X + Y where Y is a positive number would cause the tokenTime[token] of the depositor to become negative. In turn, this would make them eligible for rewards without any deposits on the protocol as the subtraction performed in getEarnedInOneDepositedIterate would become an addition, providing "free" rewards to the account.

Impact:

As a negative tokenTime equals acquisition of rewards without any assets deposited, the accounting system of the contract can be corrupted leading to users being unable to withdraw their funds. The Bolide team should inspect if this issue has been actively exploited in their active deployment and remediate it by transmitting the necessary BLID funds to accommodate for the exploited rewards.

Example:

Copy
232function withdraw(uint256 amount, address token) external isUsedToken(token) whenNotPaused {
233    uint8 decimals = AggregatorV3Interface(token).decimals();
234    uint256 countEarns_ = countEarns;
235    uint256 amountExp18 = amount * 10**(18 - decimals);
236    DepositStruct storage depositor = deposits[msg.sender];
237    require(depositor.amount[token] >= amountExp18 && amount > 0, "E4");
238    if (amountExp18 > tokenBalance[token]) {
239        ILogicContract(logicContract).returnToken(amount, token);
240        interestFee(msg.sender);
241        IERC20Upgradeable(token).safeTransferFrom(logicContract, msg.sender, amount);
242        tokenDeposited[token] -= amountExp18;
243        tokenTime[token] -= int256(block.timestamp * (amountExp18));
244    } else {
245        interestFee(msg.sender);
246        IERC20Upgradeable(token).safeTransfer(msg.sender, amount);
247        tokenTime[token] -= int256(block.timestamp * (amountExp18));
248
249        tokenBalance[token] -= amountExp18;
250        tokenDeposited[token] -= amountExp18;
251    }
252    if (depositor.depositIterate[token] == countEarns_) {
253        depositor.tokenTime[token] -= int256(block.timestamp * (amountExp18));
254    } else {
255        depositor.tokenTime[token] =
256            int256(depositor.amount[token] * earnBLID[countEarns_ - 1].timestamp) -
257            int256(block.timestamp * (amountExp18));
258        depositor.depositIterate[token] = countEarns_;
259    }
260    depositor.amount[token] -= amountExp18;
261
262    // Claim BoostingRewardBLID
263    _claimBoostingRewardBLIDInternal(msg.sender, true);
264
265    emit UpdateTokenBalance(tokenBalance[token], token);
266    emit Withdraw(msg.sender, token, amountExp18);
267}

Recommendation:

The issue arises from the fungibility of tokens as no two units are uniquely discernible. As a result, a "fair" way to remediate this issue would be too expensive to implement on-chain.

As a viable solution, the contract can prohibit withdrawals when depositor.depositIterate[token] == countEarns_ is true. In the case that more than a single earn epoch has passed since the last deposit, the user is able to do a proper withdrawal as the tokenTime could be reset to (depositor.amount[token] - amountExp18) * earnBLID[countEarns_ - 1].timestamp fairly.

Alternatively, a more penalizing approach would be to subtract tokenTime by the last earnBLID timestamp multiplied by the withdrawal amount rather than the current timestamp. As a higher tokenTime means less rewards for the user based on getEarnedInOneDepositedIterate, a subtraction of the "minimum" amount would guarantee that a user is being rewarded exactly or less than their intended amount. This would require the getEarnedInOneDepositedIterate function to also yield 0 if its calculation would underflow to ensure that users aren't provided with "negative" rewards / rewards that cast to a significantly high number.

Alleviation (fcd1a542d2):

The Bolide Finance team provided test cases that are meant to nullify this exhibit, however, they do not properly cover the exploitation scenario. As described in the exhibit, a negative value itself is an issue as it will cause the getEarnedInOneDepositedIterate function to perform an addition (- thisDepositor.tokenTime[token] * thisEarnBLID.rates[token].toInt256() of L825-L827 in the revised commit hash). As a result, the user would acquire rewards incorrectly.

Alleviation (c2c03f7731):

The Bolide Finance team produced a document detailing extensively how the calculations performed in the formula are functioning as expected. Their final assessment contains an assumption that x1 > x2 which is not correct as x1 (amount deposited) can be equal to x2 (amount withdrawn). In this edge case, x1 * t1 - x2 * t2 will be negative as x1 == x2 and t1 < t2.

Alleviation (c2c03f7731):

The Bolide Finance team stated that they believe their formulas to be executing as expected and that they cannot identify the flaw we are describing, instead categorizing it as expected behaviour. At the behest of the Bolide Finance team we will consider this exhibit as acknowledged.

SEG-13M: Incorrect Timestamp Delta Evaluation

Type	Severity	Location
Logical Fault		Storage.sol:L685

Description:

The updateAccumulatedRewardsPerShareById function states that unchecked arithmetics are used as accumulatedRewardsPerShare[token][id - 1] equals zero, however, earnBLID[id - 1].timestamp will also equal zero which will cause the thisEarnBLID.timestamp to be multiplied by allBLID as if rewards were being counted from the beginning of Unix time (January 1st 1970 00:00 UTC).

Impact:

Presently, if the first earnBLID has a non-zero allBLID value rewards will be disproportionately calculated distributing a significant level of rewards to the first epoch in comparison to other epochs.

Example:

Copy
678function updateAccumulatedRewardsPerShareById(address token, uint256 id) private {
679    EarnBLID storage thisEarnBLID = earnBLID[id];
680    //unchecked is used because if id = 0 then  accumulatedRewardsPerShare[token][id-1] equal zero
681    unchecked {
682        accumulatedRewardsPerShare[token][id] =
683            accumulatedRewardsPerShare[token][id - 1] +
684            ((thisEarnBLID.allBLID *
685                (thisEarnBLID.timestamp - earnBLID[id - 1].timestamp) *
686                thisEarnBLID.rates[token]) / thisEarnBLID.tdt);
687    }
688}

Recommendation:

As rewards for the first epoch should be counted, the updateAccumulatedRewardsPerShareById function should have a special case whereby if id is 0 a special "start-of-rewards" timestamp is utilized to calculate the timestamp delta between the start of the contract's operation and the first epoch's conclusion.

Alleviation (fcd1a542d2d7b23c561a73199ec91be0a8f0a340):

The Bolide Finance team opted to assign a value of 0 to the first accumulatedRewardsPerShare entry, ensuring that the ensuing cumulative rewards per share values are properly calculated by taking into account the timestamp of the first epoch as the start of rewards in the system. Based on these changes, we consider this exhibit adequately alleviated.

SEG-14M: Incorrect Token Time Calculations

Type	Severity	Location
Logical Fault		Storage.sol:L645-L650, L657

Description:

The depositInternal contains a code path whereby a token time delta per user will not be accounted for in the total token time of a token. This occurs when the depositIterate of a token does not match the current one and the tokenTime is "reset" via an overwrite. As a result, the user will be eligible for more rewards than originally intended.

Impact:

It is possible to acquire more rewards than intended due to the way the tokenTime is accounted in the contract. As a result, certain users who have not claimed their rewards / have withdrawn their BLID yet may be unable to do so. The Bolide team should inspect if this issue has been actively exploited in their active deployment and remediate it by transmitting the necessary BLID funds to accommodate for the exploited rewards.

Example:

Copy
625function depositInternal(
626    uint256 amount,
627    address token,
628    address accountAddress
629) internal {
630    require(amount > 0, "E3");
631    uint8 decimals = AggregatorV3Interface(token).decimals();
632    DepositStruct storage depositor = deposits[accountAddress];
633    IERC20Upgradeable(token).safeTransferFrom(msg.sender, address(this), amount);
634    uint256 amountExp18 = amount * 10**(18 - decimals);
635    if (depositor.tokenTime[address(0)] == 0) {
636        depositor.iterate = countEarns;
637        depositor.depositIterate[token] = countEarns;
638        depositor.tokenTime[address(0)] = 1;
639        depositor.tokenTime[token] += int256(block.timestamp * (amountExp18));
640    } else {
641        interestFee(accountAddress);
642        if (depositor.depositIterate[token] == countEarns) {
643            depositor.tokenTime[token] += int256(block.timestamp * (amountExp18));
644        } else {
645            depositor.tokenTime[token] = int256(
646                depositor.amount[token] *
647                    earnBLID[countEarns - 1].timestamp +
648                    block.timestamp *
649                    (amountExp18)
650            );
651
652            depositor.depositIterate[token] = countEarns;
653        }
654    }
655    depositor.amount[token] += amountExp18;
656
657    tokenTime[token] += int256(block.timestamp * (amountExp18));
658    tokenBalance[token] += amountExp18;
659    tokenDeposited[token] += amountExp18;
660
661    // Claim BoostingRewardBLID
662    _claimBoostingRewardBLIDInternal(accountAddress, true);
663
664    emit UpdateTokenBalance(tokenBalance[token], token);
665    emit Deposit(accountAddress, token, amountExp18);
666}

Recommendation:

We advise the delta to be properly accounted for in the tokenTime[token] value to ensure calculations for BLID earn periods are properly conducted. To achieve this efficiently, the value of block.timestamp * (amountExp18) can be stored to a local variable that is overwritten in the double-else branch of the depositInternal code by the proper tokenTime delta that occurs due to the reset.

Alleviation (fcd1a542d2):

After extensive discussions with the Bolide Finance team, we initially concluded that this exhibit is inapplicable. After a secondary review, we would like to request the Bolide Finance team to ensure the code behaves as expected if a user re-deposits after 2 (or more) epochs have passed. This should cause the user's original tokenTime to be overwritten with a larger one whose delta is not accounted for in the contract-level tokenTime value.

To illustrate the vulnerability a bit more verbosely:

• User deposits 10 units in the first epoch

• User's tokenTime is set to 10 * block.timestamp[A] & global tokenTime is set to 10 * block.timestamp[A]

• A second epoch is introduced

• A third epoch is introduced

• User re-deposits 10 units in third epoch

• User's tokenTime is set to 10 * block.timestamp[B] + 10 * block.timestamp[C]

• Global tokenTime is increased by 10 * block.timestamp[C]

• As block.timestamp[A] < block.timestamp[B], the delta between the original tokenTime (block.timestamp[A] * 10) and the "reset" tokenTime (block.timestamp[B] * 10) is not accounted for in the global tokenTime.

Alleviation (c2c03f7731):

The Bolide Finance team produced a document detailing extensively how the calculations performed in the formula track time correctly. After tracking all state changes that would occur on each step, we concluded that the exhibit is indeed inapplicable. As such, we consider this exhibit nullified.

SEG-15M: Inexistent Normalization of BLID Per Share

Type	Severity	Location
Mathematical Operations		Storage.sol:L316-L317, L343, L346, L350, L611, L742, L745, L765

Description:

The contract incorrectly calculates a reward per share value as it does not account for the presence of BLID deposits per user. As a result, the reward per block will be distributed multiplicatively for each user instead of being divided by all users having staked their assets on the contract.

Impact:

The current reward mechanism will lead to a rapid inflationary action for the BLID token as it would distribute a significant number of rewards to each user staked regardless of their actual stake in the protocol, breaking the incentive mechanisms commonly associated with staking protocols and providing a "flat" reward for each staked unit.

Example:

Copy
759function _boostingUpdateAccBlidPerShare() internal {
760    if (block.number <= lastRewardBlock) {
761        return;
762    }
763
764    uint256 passedblockcount = block.number - lastRewardBlock;
765    accBlidPerShare = accBlidPerShare + (passedblockcount * blidPerBlock);
766    lastRewardBlock = block.number;
767}

Recommendation:

We advise the contract to account the "active" blidDeposit values (without taking into account the blidOverDeposit) in a dedicated variable similarly to totalSupplyBLID. This variable should consequently be utilized in all accBlidPerShare calculations (i.e. getBoostingClaimableBLID, _boostingUpdateAccBlidPerShare) to divide the new reward by the total active BLID deposited to the contract, ensuring new rewards are properly and evenly distributed to each user staked using BLID tokens.

Alleviation (fcd1a542d2):

The Bolide Finance team revised their distribution formula using a better approach, however, the code still suffers from the inexistent share normalization. The accBlidPerShare is meant to illustrate the reward units per staked BLID unit. As such, the value must represent whatever we wish to distribute per block, multiplied by the blocks that have passed and divided by the total stake units eligible for a reward. In the current implementation, users will acquire a significantly disproportionate reward. For more information, consult the relevant code of MasterChef, a widely renowned staking implementation. As one can observe, they calculate the sushiReward as the reward per block multiplied by the total blocks that have elapsed and they divide that by all eligible stake units (lpSupply in SushiSwap, activeSupplyBLID in Bolide Finance's code). We advise this paradigm to be replicated in the _boostingUpdateAccBlidPerShare function's code.

Alleviation (c2c03f7731):

The Bolide Finance team stated that the expected reward of the system is not represented by passedblockcount * blidPerBlock but rather by passedblockcount * blidPerBlock * min(activeSupplyBLID, maxActiveBLID). As a result, the new equation present in the codebase is correct. We would like to note that the new equation described represents a significantly large and fluctuating reward. To ensure a more standardized reward system with easier to handle values, we would advise the usage of passedblockcount * blidPerBlock as the "maximum" reward per block with a multiplication being performed with activeSupplyBLID / maxActiveBLID. In any case, the current implementation is correct and satisfies the business requirements of Bolide Finance thus rendering this exhibit alleviated.