Skip to content

Commit 4794be6

Browse files
wadealexcypatil12
authored andcommitted
fix: dont call strategy if we have no burnable shares (#1106)
**Motivation:** Fixes an issue arbitrary external contracts could be called via `StrategyManager.burnShares`. (Certora L-04) **Modifications:** `StrategyManager.burnShares` does not do an external call if the burnable share amount is zero **Result:** Should no longer be possible to call untrusted code directly through `burnShares`
1 parent af0fbb3 commit 4794be6

File tree

2 files changed

+8
-2
lines changed

2 files changed

+8
-2
lines changed

docs/core/StrategyManager.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ function burnShares(
298298

299299
Anyone can call this method to burn slashed shares previously added by the `DelegationManager` via `increaseBurnableShares`. This method resets the strategy's burnable shares to 0, and directs the corresponding `strategy` to convert the shares to tokens and transfer them to `DEFAULT_BURN_ADDRESS`, rendering them unrecoverable.
300300

301+
The `strategy` is not called if the strategy had no burnable shares.
302+
301303
*Effects*:
302304
* Resets the strategy's burnable shares to 0
303305
* Calls `withdraw` on the `strategy`, withdrawing shares and sending a corresponding amount of tokens to the `DEFAULT_BURN_ADDRESS`

src/contracts/core/StrategyManager.sol

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,12 @@ contract StrategyManager is
158158
(, uint256 sharesToBurn) = EnumerableMap.tryGet(burnableShares, address(strategy));
159159
EnumerableMap.remove(burnableShares, address(strategy));
160160
emit BurnableSharesDecreased(strategy, sharesToBurn);
161-
// burning shares is functionally the same as withdrawing but with different destination address
162-
strategy.withdraw(DEFAULT_BURN_ADDRESS, strategy.underlyingToken(), sharesToBurn);
161+
162+
// Burning acts like withdrawing, except that the destination is to the burn address.
163+
// If we have no shares to burn, we don't need to call the strategy.
164+
if (sharesToBurn != 0) {
165+
strategy.withdraw(DEFAULT_BURN_ADDRESS, strategy.underlyingToken(), sharesToBurn);
166+
}
163167
}
164168

165169
/// @inheritdoc IStrategyManager

0 commit comments

Comments
 (0)