Anybody can withdraw underlying asset without ERC5095 token burn before maturity without burning leading to theft of funds

Summary

The current withdraw logic of ERC5095.withdraw() allows a token owner to be able to withdraw without burning token before maturity

Vulnerability Detail

The ERC5095.withdraw() is meant to allow a token (ERC5095) owner or approved caller to withdraw the asset of the underlying token pre-maturity or on/after maturity.

However the pre-maturity logic allows for withdrawal of underlying asset by anybody, owner or approved caller without any token burining and thereby having all risks borne by the contract thereby which leads to theft and loss of funds. This is due to the direct selling of principal token and transfer to receiver without the _burn call.

Additionally, it is for owner and an approved owner to collaborate and steal funds from the contract with this pre-maturity logic by calling ERC20Permit.permit() to set allowance before each withdraw call pre-maturity.

Impact

Loss of funds

Code Snippet

Pre-maturity logic https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L209-L228

function withdraw(
        uint256 a,
        address r,
        address o
    ) external override returns (uint256) {
        // Pre maturity
        if (block.timestamp < maturity) {
            uint128 shares = Cast.u128(previewWithdraw(a));
            // If owner is the sender, sell PT without allowance check
            if (o == msg.sender) {
                uint128 returned = IMarketPlace(marketplace).sellPrincipalToken(
                    underlying,
                    maturity,
                    shares,
                    Cast.u128(a - (a / 100))
                );
                Safe.transfer(IERC20(underlying), r, returned);
                return returned;
                // Else, sell PT with allowance check
            } else {
                uint256 allowance = _allowance[o][msg.sender];
                if (allowance < shares) {
                    revert Exception(
                        20,
                        allowance,
                        shares,
                        address(0),
                        address(0)
                    );
                }
                _allowance[o][msg.sender] = allowance - shares;
                uint128 returned = IMarketPlace(marketplace).sellPrincipalToken(
                    underlying,
                    maturity,
                    Cast.u128(shares),
                    Cast.u128(a - (a / 100))
                );
                Safe.transfer(IERC20(underlying), r, returned);
                return returned;
            }
        }

Assume Alice does not own any token from the ERC5095 token contract.
Assume the underlying asset is WETH
Alice then calls ERC5095.withdraw() function with the following arguments: s - 10000e18 - 10000 tokens (assuming decimals() is 18) r - Alice's address o - Alice's address
Assume amount returned after selling PT is 10000e18
Alice would receive 10000 WETH for free

Tool used

Manual Review

Recommendation

Implement the _burn for pre-maturity logic to ensure the caller actually pays back to the contract while withdrawing before maturity.

Duplicate of #195

sherlock-audit / 2022-10-illuminate-judging

cryptphi - Anybody can withdraw underlying asset without ERC5095 token burn before maturity without burning leading to theft of funds #162