Allowance check always true in ERC5095 redeem

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-illuminate/blob/217ddfef05bc9df5c7b94f1c3226a46ee136b57d/marketplace/ERC5095.sol#L116

Vulnerability details

Impact

In redeem, it is checked that the allowance is larger than underlyingAmount, which is the return parameter (i.e., equal to 0 at that point). Therefore, this check is always true and there is no actual allowance check, allowing anyone to redeem for another user.

Recommended Mitigation Steps

Change the underlyingAmount to principalAmount, which is the intended parameter.

[sourabhmarathe]

While we did not actually intend to audit the 5095 implementation, as 5095 itself is not yet final, we did describe its purpose in our codebase in the initial readme, and didn't specify that it was not in scope.
(we wanted wardens to understand its role in our infra)

With that context, we will leave it up to the judges whether or not to accept issues related to the ERC5095 token.

[gzeoneth]

I think it is fair to accept issues related to the ERC5095 token.