Miners with substantial power can prematurely seizure loans

[mrthankyou]

Description of the Bug

The PawnBank contract seizeNFT function requires that the the loanCompletionTime be less than the block's timestamp:

require(loan.loanCompleteTime < block.timestamp, "Can't seize before expiry.");

Unfortunately, the code is using block.timestamp to determine the current block timestamp. This can be manipulated by a major block miner who if has enough ming power can insert their own transaction that manipulates the block.timestamp value in such a way that they can seize a NFT loan before it expires.

Solution

Do not use block.timestamp to determine if a loan is expired. Unfortunately I do not have a strong suggestion for a fix. I would be interested in what ideas you do have to fix this.

Anything Else We Should Know

• Block Timestamp Manipulation Attack Article

[Anish-Agnihotri]

Great catch, using timestamp space over block time-space was an active decision I made after looking around a bit.

The context being that for a fixed-term loan product where it is important for a borrower to know exactly when their loan expires, using a timestamp is more accurate than working with variable block times.

My understanding is that block.timestamp has never been manipulated before, there is a check in geth to ensure miners can't shift the block-time >15 seconds, and the Ethereum wiki block validation protocol describes the maximum shift as 900 seconds.

Perhaps the easy solution is to keep the block timestamp, but add a constant 15-minute (900 second) buffer window to all loans, where interest is not accrued, at the expense of the lender. This would scale poorly for very short-dated, high-interest loans, but I doubt such short-termed loans will be common via PawNFT.

[mrthankyou]

Yeah, this is a tough one. On one hand block.timestamp is tough to manipulate. You have to find a solution that is elegant but also solves the security issue, in this case not utilizing block.timestamp. I honestly don't have a good answer or solution for you. For now I feel like at least knowing this is an issue is helpful and may be good enough. If/when you decide to move this to mainnet, then one can review this issue again. That's my two cents anyways.

[Anish-Agnihotri]

Right, makes sense. Will leave this issue open for anyone who wants to adapt the contracts for their own use.