QA Report

[code423n4]

QA

1.

Title: Missing checks tokens.length == _tokenIds.length

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Basket.sol#L41

We should've check that every token withdraw has tokenId and token address in pair for batch ERC721 batch transfer to prevent 0 tokenId or address(0) token transfer

Recommended mitigation step

require(_tokens.length == _tokenId.length);

2.

Title: Usage address.transfer

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Basket.sol#L80

The transfer function has a tight gas restriction and might fail if GAS costs change in the future or if a smart contract's fallback function handler is complex. I recommend to use address.call()

Tool used

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Recommended mitigation step

Use address.call()

3.

Title: Using SafeTransfer library for error handling

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Basket.sol#L87 https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Basket.sol#L94

Apart from synthetic tokens which have been engineered to always return true, consider using OpenZeppelin's SafeERC20 library for transferring ERC20 token. Some token won't return bool value (USDT for example)

[HardlyDifficult]

Good suggestions to consider.