1Hive / time-lock-app

The Time Lock app enables Aragon organizations to require users transfer or temporarily lock tokens before forwarding an intent.
GNU General Public License v3.0
16 stars 11 forks source link

TimeLock spam prevention can be bypassed #83

Closed wadealexc closed 4 years ago

wadealexc commented 4 years ago

Description

The TimeLock app is a forwarder that requires users to lock some token before forwarding an EVM callscript. Its purpose is to introduce a "spam penalty" to hamper repeat actions within an Aragon org. In the context of a Dandelion org, this spam penalty is meant to stop users from repeatedly creating votes in DandelionVoting, as subsequent votes are buffered by a configurable number of blocks (DandelionVoting.bufferBlocks). Spam prevention is important, as the more votes are buffered, the longer it takes before "non-spam" votes are able to be executed.

By allowing arbitrary calls to be executed, the TimeLock app opens several potential vectors for bypassing spam prevention.

Examples

  1. Using a callscript to transfer locked tokens to the sender

By constructing a callscript that executes a call to the lock token address, the sender execute calls to the lock token on behalf of TimeLock. Any function can be executed, making it possible to not only transfer "locked" tokens back to the sender, but also steal other users' locked tokens by way of transfer.

  1. Using a batched callscript to call DandelionVoting.newVote repeatedly

Callscripts can be batched, meaning they can execute multiple calls before finishing. Within a Dandelion org, the spam prevention mechanism is used for the DandelionVoting.newVote function. A callscript that batches multiple calls to this function can execute newVote several times per call to TimeLock.forward. Although multiple new votes are created, only one spam penalty is incurred, making it trivial to extend the buffer imposed on "non-spam" votes.

  1. Using a callscript to re-enter TimeLock and forward or withdrawAllTokens to itself

A callscript can be used to re-enter TimeLock.forward, as well as any other TimeLock functions. Although this may not be directly exploitable, it does seem unintentional that many of the TimeLock contract functions are accessible to itself in this manner.

Recommendation

  1. Add the TimeLock contract's own address to the evmscript blacklist
  2. Add the TimeLock lock token address to the evmscript blacklist
  3. To fix spamming through batched callscripts, one option is to have users pass in a destination and calldata, and manually perform a call.
fabriziovigevani commented 4 years ago

closed by #84