You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jun 30, 2024. It is now read-only.
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelMediumA valid Medium severity issueRewardA payout will be made for this issue
Using incorrect function to determine the token supply in a Balancer weighted pool
Summary
The protocol uses the totalSupply() function to obtain the number of LP tokens in a Balancer weighted pool. However, this function does not always return the correct count, and the Balancer documentation recommends using getActualSupply() instead.
Vulnerability Detail
/** * @notice Returns the effective BPT supply. * * @dev This would be the same as `totalSupply` however the Pool owes debt to the Protocol in the form of unminted * BPT, which will be minted immediately before the next join or exit. We need to take these into account since, * even if they don't yet exist, they will effectively be included in any Pool operation that involves BPT. * * In the vast majority of cases, this function should be used instead of `totalSupply()`. * * **IMPORTANT NOTE**: calling this function within a Vault context (i.e. in the middle of a join or an exit) is * potentially unsafe, since the returned value is manipulable. It is up to the caller to ensure safety. * * This is because this function calculates the invariant, which requires the state of the pool to be in sync * with the state of the Vault. That condition may not be true in the middle of a join or an exit. * * To call this function safely, attempt to trigger the reentrancy guard in the Vault by calling a non-reentrant * function before calling `getActualSupply`. That will make the transaction revert in an unsafe context. * (See `whenNotInVaultContext` in `WeightedPool`). * * See https://forum.balancer.fi/t/reentrancy-vulnerability-scope-expanded/4345 for reference. */function getActualSupply() externalviewreturns (uint256) {
uint256 supply =totalSupply();
(uint256protocolFeesToBeMinted, ) =_getPreJoinExitProtocolFees(
getInvariant(),
_getNormalizedWeights(),
supply
);
return supply.add(protocolFeesToBeMinted);
}
From the comment for the getActualSupply() function in the Balancer source code, it is apparent that this function should be used instead of totalSupply() because the pool owes debt to the protocol in the form of unminted BPT, which should be taken into consideration. I leave here a link to a similar issue found a few weeks ago: sherlock-audit/2023-10-notional-judging#36
2 submodules are affected BalancerPoolTokenPrice and AuraBalancerSupply.
Impact
Potentially incorrect prices returned by the Balancer feed, which could impact the operation of the RBS module. For instance, using the wrong price during a swap may lead to financial losses for the protocol.
The text was updated successfully, but these errors were encountered:
sherlock-admin
changed the title
Nice Merlot Hare - Interface Assumptions
Formal White Beaver - Using incorrect function to determine the token supply in a Balancer weighted pool
Dec 28, 2023
sherlock-admin2
changed the title
Formal White Beaver - Using incorrect function to determine the token supply in a Balancer weighted pool
ge6a - Using incorrect function to determine the token supply in a Balancer weighted pool
Jan 8, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelMediumA valid Medium severity issueRewardA payout will be made for this issue
ge6a
medium
Using incorrect function to determine the token supply in a Balancer weighted pool
Summary
The protocol uses the totalSupply() function to obtain the number of LP tokens in a Balancer weighted pool. However, this function does not always return the correct count, and the Balancer documentation recommends using getActualSupply() instead.
Vulnerability Detail
From the comment for the getActualSupply() function in the Balancer source code, it is apparent that this function should be used instead of totalSupply() because the pool owes debt to the protocol in the form of unminted BPT, which should be taken into consideration. I leave here a link to a similar issue found a few weeks ago: sherlock-audit/2023-10-notional-judging#36
2 submodules are affected BalancerPoolTokenPrice and AuraBalancerSupply.
Impact
Potentially incorrect prices returned by the Balancer feed, which could impact the operation of the RBS module. For instance, using the wrong price during a swap may lead to financial losses for the protocol.
Code Snippet
https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/BalancerPoolTokenPrice.sol#L402
https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/AuraBalancerSupply.sol#L345
Tool used
Manual Review
Recommendation
Use getActualSupply() instead totalSupply() for all Balancer pools that support it.
Duplicate of #155
The text was updated successfully, but these errors were encountered: