0x52 - TxBuilderExtension#redeemNativeToken fails to accrueToken before getSupplyBalance

[sherlock-admin]

0x52

medium

TxBuilderExtension#redeemNativeToken fails to accrueToken before getSupplyBalance

Summary

TxBuilderExtension#redeemNativeToken fails to accrueToken before getSupplyBalance when the user is attempting to fully redeem. This leads to funds being left in the contract that must be recovered with a subsequent transaction. On mainnet, the gas cost to recover these funds are more than they are worth leading to loss to the user.

Vulnerability Detail

TxBuilderExtension.sol#L275-L283

function redeemNativeToken(address user, uint256 redeemAmount) internal nonReentrant {
    if (redeemAmount == type(uint256).max) {
        redeemAmount = ironBank.getSupplyBalance(user, weth);
    }
    ironBank.redeem(user, address(this), weth, redeemAmount);
    WethInterface(weth).withdraw(redeemAmount);
    (bool sent,) = user.call{value: redeemAmount}("");
    require(sent, "failed to send native token");
}

Above we see that ironBank.getSupplyBalance is called without first accruing interest. This leads to an out-dated supply balance and all pending interest will be left deposited to the user. While a user can recover these funds at anytime, the gas costs (especially on mainnet) to recover them can easily be larger than the amount that is left deposited. This means that the funds left deposited are economically lost to the user due to a malfunction of the smart contract.

Impact

User funds are left deposited requiring the user to spend more in gas to recover it than it is worth

Code Snippet

TxBuilderExtension.sol#L275-L283

Tool used

Manual Review

Recommendation

Accrue interest to weth market before getting supply balance.

[ibsunhub]

Agree with the issue, and we will fix this. However, we believe the severity can be discussed. The effect of this issue at most is that the user can't redeem full only for native token. No funds will be lost.

[0xffff11]

Agree with sponsors issue validity. I see the point where the issue could be low. Watsons specified impact is not worth a medium imo.

[0xffff11]

Low

[IAm0x52]

Escalate for 10 USDC

This should be medium because while funds aren't lost they are indefinitely DOS'd directly as a result of malfunctioning contract code. If it costs $30 to recover funds worth $20 then those funds are essentially locked. The user has to wait an indefinite amount of time for gas prices to drop before their funds are (potentially) economically recoverable.

[sherlock-admin]

Escalate for 10 USDC

This should be medium because while funds aren't lost they are indefinitely DOS'd directly as a result of malfunctioning contract code. If it costs $30 to recover funds worth $20 then those funds are essentially locked. The user has to wait an indefinite amount of time for gas prices to drop before their funds are (potentially) economically recoverable.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ibsunhub]

The funds are not locked. Users could still redeem fully as WETH.

[IAm0x52]

My point is that even though it can be redeemed the user would pay more for gas fees than they would get from withdrawing it. This gives the user two options. Either they withdraw it and lose money after gas fees or they hope and wait for gas fees to be low enough so they don't lose money, meaning the user is DOS'd indefinitely.

[0xffff11]

I see the watsons point. I still believe that it should be a low though.

"If it costs $30 to recover funds worth $20 then those funds are essentially locked."

This is a quite extreme example where gas fees are 30$ (while in the future it might be a time where this is true, it is not reasonable) and a low amount of 20$ locked.

To me, I still see it as a low

[IAm0x52]

I disagree that that gas fee is unreasonable on mainnet. Right now fees are lower but in the past sustained high gas fees for long periods of time was the norm. Gas prices only recently got cheap and with increases network activity those fees will go right back up.

[0xffff11]

I do not remember exactly where, might have been deleted, but I think I saw that if funds are stuck, but user can redeem them 1:1 in less than let's say 100 days, it was considered a low. Here I see the same, no funds are lost, and yes gas can be very expensive for long periods of time, but users will definitely get the change of redeeming at a decent gas cost.
Also, as sponsor signaled, I believe issue #499 and the duplicates are duplicates. They have the same attack surface, not accruing debt, this one is signaling that user will have to redeem funds after. Therefore, if this one is judged as a low, the respective issues should also be a low.

As explained by sponsor, the accrual model they used is similar to the one from compound and it is a tradeOff between gas fees.

To be perfectly honest, I am tending between both medium and low, but more to low, given the sponsor reasoning.

[ibsunhub]

fix: ibdotxyz/ib-v2#48

[hrishibhat]

Result:
Low
Has duplicates
Considering this a valid low issue based on the Sponsors comment as the impact is not being significant enough.

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• IAm0x52: rejected

[IAm0x52]

Fix looks good. Interest is now accrued as expected