sherlock-audit / 2024-04-interest-rate-model-judging

2 stars 1 forks source link

bareli - cancel function in EscrowedEXA.sol will always revert. #240

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

bareli

medium

cancel function in EscrowedEXA.sol will always revert.

Summary

cancel will always revert as we are deleting reserves[streamId] first then we are calling withdrawMax(streamId) which calls checkStream which will revert if its zero.

Vulnerability Detail

function cancel(uint256[] memory streamIds) external returns (uint256 streamsReserves) { uint128 refundableAmount; for (uint256 i = 0; i < streamIds.length; ++i) { uint256 streamId = streamIds[i]; checkStream(streamId); assert(msg.sender == sablier.getRecipient(streamId)); stream reserves += reserves[streamId]; @>> delete reserves[streamId]; refundableAmount += sablier.refundableAmountOf(streamId); @>> withdrawMax(streamId); sablier.cancel(streamId); } emit Cancel(msg.sender, streamIds); _mint(msg.sender, refundableAmount); exa.safeTransfer(msg.sender, streamsReserves); }

function withdrawMax(uint256[] memory streamIds) public { for (uint256 i = 0; i < streamIds.length; ++i) { uint256 streamId = streamIds[i]; @> checkStream(streamId); assert(msg.sender == sablier.getRecipient(streamId)); withdrawMax(streamId); } }

function checkStream(uint256 streamId) internal view { if (reserves[streamId] == 0) revert InvalidStream(); }

Impact

we cannot cancel any streamid as its always revert.

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/periphery/EscrowedEXA.sol#L133

Tool used

Manual Review

Recommendation

we should delete the streamId at the last.

function cancel(uint256[] memory streamIds) external returns (uint256 streamsReserves) { uint128 refundableAmount; for (uint256 i = 0; i < streamIds.length; ++i) { uint256 streamId = streamIds[i]; checkStream(streamId); assert(msg.sender == sablier.getRecipient(streamId)); stream reserves += reserves[streamId];

  refundableAmount += sablier.refundableAmountOf(streamId);

@>> withdrawMax(streamId); delete reserves[streamId]; sablier.cancel(streamId); } emit Cancel(msg.sender, streamIds); _mint(msg.sender, refundableAmount); exa.safeTransfer(msg.sender, streamsReserves); }

santipu03 commented 1 month ago

This issue is invalid because the withdrawMax function that is called through cancel is an internal function, not the public one despite having the same names.