Summary

_isEscrowed function just performs a zero address check, doesn't make sure that the address is actually the msg.sender.
The Starklane::withdrawTokens function calls StarklaneEscrow::_withdrawFromEscrow which calls StarklaneEscrow::_isEscrowed to check if the address is present in the escrow or not. But it doesn't account if the address is actually of the right owner or not.

Vulnerability Details

withdrawTokens function

_withdrawFromEscrow function

_isEscrowed function

depositTokens function

In withdrawTokens function, this line:

The above line will always return true , because this calls _withdrawFromEscrow function checks if the address is escrowed or not with _isEscrowed function.

_isEscrowed function just checks if the address entered is greater than 0 or not, any address can be passed other than 0 address and it will return true. There is no actual checks present to ensure the address entered is indeed a legitimate escrowed address.

Hence bool wasEscrowed will always be true.

This part from withdrawTokens function will never be executed. And minting will never happen in case the tokens are not present in the escrow.

No checks present in withdrawTokens function to ensure that the tokens can only be withdrawn by the right owner.
Anyone who has deposited once using Starklane::depositTokens function will have their id in escrow as Starklane::depositTokens function updates the id to corresponding msg.sender here :

Hence they can pass the check performed by _isEscrowed easily as their id is already present in escrow. Anyone should be able to take out tokens from escrow without being the owner.

Impact

In case the tokens were not present in the escrow it will not be minted as wasEscrowed will always be true.
Anyone can withdraw anyone else's tokens.

Tools Used

Manual review

Recommendations

In my opinion, this check should be performed along with the one which is already being done in _isEscrowed function which would ensure that only the rightful owner can withdraw funds.