Validations aren't sufficient to prevent reorg issues

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/BuyCreditMarket.sol#L109-L111 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L116-L118

Vulnerability details

Impact

When a user submits a market sell credit tx (or a market buy credit tx), they specify the maximum (or minimum) APR that they are willing to accept. This appears to be useful because the counterparty may change their offer right before the tx executes, so params.maxAPR (and params.minAPR) help ensure on-chain that the user is actually willing to accept the loan.

Note that in the current code, there is no similar check that the length of the loan is acceptable for the msg.sender. This is obviously not necessary in the case where params.creditPositionId == RESERVED_ID, since the length of the loan is determined by the user-supplied params.tenor anyway. In the case where params.creditPositionId != RESERVED_ID, it may not appear necessary to validate the length of the loan, since the user-supplied params.creditPositionId will have a preset deadline that can't be changed.

However, there is one situation where the params.creditPositionId != RESERVED_ID case can go wrong. In most blockchains, there are scenarios where reorgs retroactively rearrange the order of blocks from what the user saw when they submitted their transaction. In these situations, it's possible that params.creditPositionId represents a completely different credit position compared to when the user sent their market sell/buy credit transaction. In this case, the user's transaction might succesfully execute (assuming the APR is acceptable) with a loan deadline that is much different than they expect.

For example of reorgs on Polygon, see: https://polygonscan.com/blocks_forked.

Proof of Concept

• In block n, Alice creates a credit position that has ID x
• In block n+1, Bob tries to buy some of Alice's credit from position x by submitting params.creditPositionId == x
• A reorg happens, and block n no longer happened. Instead Charlie's credit position is created in block n with ID x. Charlie's credit position is with a much further deadline.
• Now, Bob's transaction can still succeed in purchasing credit ID x, but it will actually be Charlie's credit position with an unexpected deadline.

Tools Used

Manual analysis.

Recommended Mitigation Steps

In the case where params.creditPositionId != RESERVED_ID, add a check that the params.tenor matches the deadline of the credit position. The params.tenor is currently unused in this scenario anyways, so this should be strictly better.

Assessed type

Invalid Validation

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid