incentive-layer icon indicating copy to clipboard operation
incentive-layer copied to clipboard

Contract JackpotManager is vulnerable to the locked ether vulnerability

Open thogiti opened this issue 7 years ago • 3 comments

I am still meditating on this issue but I have a bit uneasy feeling in the stomach about the JackpotManager. There is a possibility of locked ether vulnerability in the contract model.

As a best practice, from smart contract security perspective, in late 2018, if a contract allows users to deposit ether by calling the deposit function and it should contain a function that would allow users to withdraw the deposited ether.

thogiti avatar Aug 10 '18 18:08 thogiti

Are you sure you meant JackpotManager? Perhaps there is a misunderstanding. Tokens going into the jackpot will always eventually get rewarded on forced errors events.

hswick avatar Aug 10 '18 23:08 hswick

The jackpot should never become empty because then the incentive for verification disappears. Aren't we depositing TRU here rather than ETH?

Welcome @thogiti! Thanks for your feedback.

teutsch avatar Aug 11 '18 00:08 teutsch

Thx for the comments. I didn't get a chance to study the complete code yet. Will do in next few days/weeks.

  1. "Stuck Ether Vulnerability" is a bad name for the stuck ether/token vulnerability. We should change to a better name.
  2. Let's say the contract gets frozen from a new security vulnerability that will be discovered in few months. Now your contract is frozen and the amount in the contract is stuck.
  3. This design pattern needs to change and any contract that allows users to deposit money should have a function to withdraw or destroy the deposited amount and the rest of the flow and business logic needs to accommodate the additional withdraw/destroy function.
  4. Jackpot doesn't have to become empty.

thogiti avatar Aug 12 '18 05:08 thogiti