c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmDeclarations.sol#L274-L354

Vulnerability details

Impact

Loss of protocol and user funds

Proof of Concept

Wiselending Protocol pre-approves some inter-protocol addresses in the PendlePowerFarmDeclarations contract´s constructor to provide smooth function flows and save gas.

However, this is an irreversible implementation as the approvals given to the addresses are not only the Wiselending contracts and they can´t be zeroed back.

It´s performed in the constructor by calling _doApprovals, and here´s the list of approved addresses;

Contract: PendlePowerFarmDeclarations.sol

274:     function _doApprovals(
275:         address _wiseLendingAddress
276:     )
277:         internal
278:     {
279:         if (block.chainid == ETH_CHAIN_ID) {
280:             _safeApprove(
281:                 ST_ETH_ADDRESS,
282:                 address(CURVE),
283:                 MAX_AMOUNT
284:             );
285:         }
286: 
287:         if (block.chainid == ARB_CHAIN_ID) {
288:             _safeApprove(
289:                 address(ENTRY_ASSET),
290:                 address(UNISWAP_V3_ROUTER),
291:                 MAX_AMOUNT
292:             );
293: 
294:             _safeApprove(
295:                 WETH_ADDRESS,
296:                 address(UNISWAP_V3_ROUTER),
297:                 MAX_AMOUNT
298:             );
299: 
300:             _safeApprove(
301:                 WETH_ADDRESS,
302:                 _wiseLendingAddress,
303:                 MAX_AMOUNT
304:             );
305:         }
306: 
307:         _safeApprove(
308:             PENDLE_CHILD,
309:             _wiseLendingAddress,
310:             MAX_AMOUNT
311:         );
312: 
313:         _safeApprove(
314:             ENTRY_ASSET,
315:             address(PENDLE_ROUTER),
316:             MAX_AMOUNT
317:         );
318: 
319:         _safeApprove(
320:             address(PENDLE_MARKET),
321:             PENDLE_CHILD,
322:             MAX_AMOUNT
323:         );
324: 
325:         _safeApprove(
326:             address(PENDLE_MARKET),
327:             address(PENDLE_ROUTER),
328:             MAX_AMOUNT
329:         );
330: 
331:         _safeApprove(
332:             address(ENTRY_ASSET),
333:             address(PENDLE_SY),
334:             MAX_AMOUNT
335:         );
336: 
337:         _safeApprove(
338:             address(ENTRY_ASSET),
339:             _wiseLendingAddress,
340:             MAX_AMOUNT
341:         );
342: 
343:         _safeApprove(
344:             address(PENDLE_SY),
345:             address(PENDLE_ROUTER),
346:             MAX_AMOUNT
347:         );
348: 
349:         _safeApprove(
350:             WETH_ADDRESS,
351:             address(AAVE_HUB),
352:             MAX_AMOUNT
353:         );
354:     }
355:

We want to emphasize that the given approvals will be draining the contract when the third party address is exploited or they drain the Wiselending contract deliberately. It´s technically possible that the Pendle contracts´ owner can drain Wiselending.

As can be seen on the above approvals, PENDLE ROUTER is also among the approved addresses. When we check the router addresses of the Pendle, we see that they are proxies (1, 2) and implementation will vary according to the update. Without assessing the future implementation, it makes a gamble to approve these contracts with a maximum amount.

In addition, Pendle uses 1inch network for the swap functions which might also lead to drain the funds from Wiselending via executing an arbitrary call through 1inch when the code is updated.

Moreover, there is no functionality to revoke these approvals. The pools could be paused by the securityShutdown call, however, this doesn´t prevent the funds from being withdrawn by the exploited protocols.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend off-chain on-time approvals at the UI

OR

Approve the addresses at the protocol level only for the _amount being interacted with a gas cost tradeoff on L1.

Assessed type

Other

GalloDaSballo commented 6 months ago

By definition should have been sent at Med at most (conditional on external condition)

I'm not convinced this should be considered valid though as it fails to demonstrate how the approvals will lead to loss

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

trust1995 commented 5 months ago

Conditional on unknown 3rd party contract being exploited == invalid speculation. Valid as a systemic risk in analysis.

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid

code-423n4 / 2024-02-wise-lending-findings

Infinite approvals to third parties are irrevokable and subject to be the soft belly when the dependent platform is exploited #175