burn() doesn't call ERC721 _burn()

[code423n4]

Handle

sirhashalot

Vulnerability details

Impact

The CollateralizedDebt.sol contract is a ERC721 token. It has a mint() function, which uses the underlying safeMint() function to create an ERC721 token representing a collateral position. The burn() function in CollateralizedDebt.sol should reverse the actions of mint() by burning the ERC721 token, but the ERC721 _burn() function is never called. This means a user can continue to hold their ERC721 token representing their position after receiving their funds. This is unlike the burn() function found in Bond.sol, Insurance.sol, and Liquidity.sol, which all call the _burn() function (though note the _burn() function in these other Timeswap Convenience contracts is the ERC20 _burn()).

Proof of Concept

The problematic burn() function is found in CollareralizedDebt.sol
https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/CollateralizedDebt.sol#L80-L88

Compare this function to the burn() functions defined in the other Timeswap Convenience contracts, which contain calls to _burn()

Recommended Mitigation Steps

Include the following line in the burn() function
_burn(id);

[Mathepreneur]

If decided not to burn the ERC721 token at all. The burn in this context is burning the debt and collateral locked balance in the ERC721 token.