delvtech / council

Flexible DAO governance smart contracts, developed by Delv.
Apache License 2.0
93 stars 19 forks source link

Double execution vulnerability in Timelock #47

Closed daejunpark closed 3 years ago

daejunpark commented 3 years ago

A batch of calls may be doubly executed in Timelock.

Scenario

Consider the following two scenarios where increaseTime() is executed after execute() for the same callHash:

  1. The authorized GSC contract accidentally or maliciously calls increaseTime() for the callHash that has already been executed. (Difficulty: High)
  2. OR, the GSC legitimately calls increaseTime() near the end of the original waiting period, but increaseTime() is delayed for some reason until the original waiting period passes, and immediately adversaries front-run execute() before the increaseTime() transaction. (Difficulty: Medium)

Then, callTimestamps[callHash] becomes non-zero again, and the subsequent execute() may lead to double execution of the same batch of calls.

Recommendation

Add an extra condition require(callTimestamps[callHash] != 0); in the increaseTime() function to ensure that the given callHash is active.