code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

Trying to use timestamps with blocknumbers #249

Closed code423n4 closed 2 years ago

code423n4 commented 3 years ago

Handle

goatbug

Vulnerability details

Impact

    require(bondTimestamp + ONE_DAY > block.number);

There are require statements comparing timestamps to blocknumbers.

We cannot assume one block per second, this code would not work on different chains where block times radically differ.

The effect is having processes such as settling Auction in this case, happen far to quickly or slowly.

Proof of Concept

This occurs in Basket and Auction contracts. publishNewIndex changeLicenseFee changePublisher

These functions all also incorrectly rely on block numbers to try implement timelocks. There are no garuntees on block times going forwards with networks. It sh

Tools Used

Recommended Mitigation Steps

frank-beard commented 3 years ago

bondTimestamp is a blocknumber, we will fix the naming to make this clear.

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L58

GalloDaSballo commented 2 years ago

Invalid finding it's all blocknumbers, definitely agree with renaming though