Sense redeem is vulnerable to reentrancy

Summary

Insufficient reentrancy protection in Redeemer allows for inflating the holdings variable via reentrancy and subsequently withdrawing more than should be allowed.

Vulnerability Detail

The Redeemer.redeem function related to Sense allows passing in an arbitrary address that will be called. This enables reentrancy by passing in a malicious contract that reenters the redeem function, which causes the holdings variable to inflate for a market.

Calling the redeem function that burns illuminate tokens and sends underlying to the sender, will then cause more tokens to be sent than the sender was eligible for.

Impact

This will leave the protocol with a deficit in underlying tokens. This can go as far as the exploiter redeeming all matured principal tokens (as the redeem functions are accessible to the public) and then taking all the underlying tokens that have been sent to the Redeemer.

Code Snippet

The redeem function for sense requires passing a Sense adapter:

@param a Sense's adapter for this market

This is used in the function like this:

ISenseDivider divider = ISenseDivider(ISenseAdapter(a).divider());

// Redeem the tokens from the Sense contract
ISenseDivider(divider).redeem(a, s, amount);

The divider.redeem function is where the control flow can be taken over and a reentrancy could be initiated.

The holdings are updated at the end of the function:

uint256 redeemed = IERC20(u).balanceOf(address(this)) - starting;

// Verify that underlying are received 1:1 - cannot trust the adapter
if (redeemed < amount) {
    revert Exception(13, 0, 0, address(0), address(0));
}

// Update the holdings for this market
//@done-audit-issue inflation through reentrancy
holdings[u][m] = holdings[u][m] + redeemed;

To exploit this, only the last reentrancy call would need to send the underlying tokens to the contract, the holdings will increase for each reentrancy call by the same amount, as the difference between starting- and balanceOf will be the same for all.

Note: an exploiting contract, would also need to make sure that the call to converter.convert does not fail by mocking the functions inside accordingly, but that raises the complexity only slightly and does not impact the exploit path.

Tool used

Manual Review

Recommendation

Add a reentrancy protection modifier such as OpenZeppelins nonreentrant. Also consider validating or whitelisting the passed contract addresses.

Duplicate of #236

sherlock-audit / 2022-10-illuminate-judging

minhtrng - Sense redeem is vulnerable to reentrancy #232