code-423n4 / 2022-07-fractional-findings

0 stars 0 forks source link

block.timestamp used as time proxy #624

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L203 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L125 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L162 https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L108 https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L154 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L194

Vulnerability details

block.timestamp used as time proxy

Impact

a. Summary:

Risk of using block.timestamp for time should be considered.

b. Details:

block.timestamp is not an ideal proxy for time because of issues with synchronization, miner manipulation and changing block times. In this case, would affect the code ending the interaction before the expected deadline and making unable to access to some functions before the expected time because they will revert.

Proof of Concept

c. Github Permalinks:

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L203 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L125 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L162 https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L108 https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L154 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L194

Recommended Mitigation Steps

d. Mitigation:

Consider the risk of using block.timestamp as time proxy and evaluate if block numbers can be used as an approximation for the application logic. Both have risks that need to be factored in.

Tools Used

Manual analysis

mehtaculous commented 2 years ago

Duplicate of #48

HardlyDifficult commented 2 years ago

Using timestamp here is reasonable. The submission did not identify a significant impact in the POC. Closing as invalid.