Kuiper contest - tensors's results

Handle

tensors

Vulnerability details

Impact

Consider adding sanity checks on these 4 methods, similar to the requirement on the setOwnerSplit method.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Factory.sol#L39-L54

Handle

tensors

Vulnerability details

I recommend adding reentrancy checks throughout Basket.sol and Auction.sol using a mutex lock. Many external calls are made to potentially unsafe token contracts. In the case that not all token contracts are properly vetted, this preventative step could be worthwhile.

Handle

tensors

Vulnerability details

Impact

In L92 of Basket.sol there is an unnecessary require check that the user balance is greater than or equal to amount. If the amount is larger than user balance then the _burn() method will fail, causing the function to revert anyway.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L92

Recommended Mitigation Steps

Remove the unnecessary check.

Handle

tensors

Vulnerability details

Throughout the code block.number is used to time functions, assuming 1 new block is added every 15 seconds. It is better to use block.timestamp for this purpose.

For example, the variable ONE_DAY in Auction.sol is given by 4 * 60 * 24, which assumes 1 block every 15 seconds. It is more natural to use the timestamp where ONE_DAY would be equal to 60 * 60 * 24 seconds.