code-423n4 / 2021-08-yield-findings

1 stars 0 forks source link

TimeLock cannot schedule the same calls multiple times #27

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The TimeLock.schedule function reverts if the same targets and data fields are used as the txHash will be the same. This means one cannot schedule the same transactions multiple times.

Impact

Imagine the delay is set to 30 days, but a contractor needs to be paid every 2 weeks. One needs to wait 30 days before scheduling the second payment to them.

Recommended Mitigation Steps

Also include eta in the hash. (Compound's Timelock does it as well.) This way the same transaction data can be used by specifying a different eta.

alcueca commented 3 years ago

Funny, BoringCrypto was quite negative about including the eta in the txHash. At the time I couldn't think of a reason to repeat the same call with the same data, but you are right that sometimes it might make sense, and storing off-chain the expected eta of each timelocked transaction is something you should do anyway.

I'll confirm this issue, and will bring it for public discussion once the contest is over.

alcueca commented 3 years ago

I ended up refactoring the Timelock so that the eta is not included in the parameters, but repeated proposals are allowed.