Cold Staking contract bug bounty. about roadmap

[yuriy77k]

@mgistrat

Function stake_reward may return the wrong value of the reward, if there were no other operations with the contract in this block. TotalStakingWeight will have old value.

As this function is used only to inform it cannot cause loss of money or incorrect behavior of the contract.
I can classify this as a minor observation, non-security issue (10,000 CLO).
Please, send your CLO address.

[mgistrat]

https://gist.github.com/mgistrat/b7c36c536e61c0d256ec4bd42cb6ff98

[Dexaran]

Callisto Security Audit is finished.

EthereumCommonwealth/Auditing#77

All the bug reports may be published in the comments below now.

[Dexaran]

@mgistrat "A staker will lose the deserved reward if he makes an additional deposit after the end of the round in case of Timestamp less then now."

This is not a bug/security vulnerability.
This situation is described in "Staking Rules": https://callisto.network/blog/post/callisto-network-cold-staking-protocol/

A staker MUST NOT deposit funds into the staking contract during the locking period. Depositing funds during the locking period will restart the locking period and staking contract.

[kabachok2]

https://gist.github.com/kabachok2/634549b48cb5e0bb96c822e44b5e4666

[yuriy77k]

@kabachok2

block.number == LastBlock, Always equal, so the condition will never be met. We will not be able to use smarkontrakt to destination.
Need to remove LastBlock initialization.

This is not a bug/security vulnerability.
uint public LastBlock = block.number; is one-time initialization during smart contract deploy.

[duychuongvn]

https://gist.github.com/betbether/187a40779c6ccb4c9931a789f78aa278

[Dexaran]

@duychuongvn

Missing check user inactive.
If user inactive more than two years, this method still return reward when she claims

If a user has come to receive his reward, it means that he has become active.

Issue 2: Method function new_block() public
The method is not payable so how this statement work?

The new_block() function is called from other functions when a user attempts to perform any action with contract. It can be called from payable function.

[duychuongvn]

@Dexaran

If a user has come to receive his reward, it means that he has become active
It 's not make sense, there are 2 methods [ function claim() public only_stake] and [ function report_abuse(address _addr) public only_staker] with same permission.

The claim() method is used to claim stake, report_abuse() method is used to withdraw stake without any reward.
Why does user call report_abuse() method while they can call claim() to get reward first, after that they call withdraw_stake()? If she is inactive more than two years, she can claim reward and withdraw stake.
Then the workflow here doesn't work:

(5) When a staker does nothing for a certain amount of time (1 year) after the locking period has ended, then they are considered inactive and are removed from the staking contract. The inactive stake is returned to the stakers address. No reward shall be paid to inactive stakers.

---

The new_block() function is called from other functions when a user attempts to perform any action with contract. It can be called from payable function.

I know we call this method from other methods in this contract, why do we public this method?

[yuriy77k]

@duychuongvn

Why does user call report_abuse() method while they can call claim() to get reward first, after that they call withdraw_stake()? If she is inactive more than two years, she can claim reward and withdraw stake.

report_abuse() is intended to return the deposit to an inactive user. It can be called by any staker who find an inactive user. Of cause, if user is active he can withdraw_stake() himself.

I know we call this method from other methods in this contract, why do we public this method?

It can be called if need to update StakingRewardPool, TotalStakingWeight, Timestamp and LastBlock for some reason.

[mgistrat]

https://gist.github.com/mgistrat/2f111ba241e398f0476c717f923babe8

[dieselc]

https://gist.github.com/dieselc/68a7544b19f107275cd126230329f527

Issue Severity: Medium

Impact: Stakers reward divided by half or more (worst case scenario).

"Medium severity issue. 200,000 CLO (~0,442 BTC) for finding security vulnerabilities and bugs,
that could not be directly exploited but can affect contracts in some specific circumstances and
can cause a loss of funds for a certain stakers."

Audited contract commit hash: 244ed1d2c3fe39d3a65d9e901a2812c3364b7c28

Scenario:

• Staker X stake at time T.
• Staker X claim is made more than 27 days after the first start_staking call.

Issue:

if the claim is made in the worst case scenario 27+26 days after the first stake request:

• _StakerWeight will represent only the first 27 days staker weight.
• TotalStakingWeight will represent the 27+26 days total weight.
  Assuming a growth of 0% since the 27th day of total staked amount the real reward will be divided by half, any higher growth will reduce the reward more.

This issue is related with all stakers that wait more than the claim period.
Other scenarios can cause the same issue. the one presented is just one case.

Solution

Calculate the whole staking weight of a staker including the whole staking period.

[dieselc]

@Dexaran @yuriy77k, I didn't think about the total reward that will double but in case of block reward reduction it will not double. I will update the gist.

[yuriy77k]

@dieselc By Cold Staking rules, staker will receive reward only for complete round interval (27 days).
If staker claim his reward after 27+26 days he receives the reward for 27 days, but he can do next claim after 1-day passed and will receive next reward for 27 days.
This is not a bug/security vulnerability.

[mgistrat]

@yuriy77k my CLO address: 0x6652576D517388e0c71745aE1f388DdD78142202

[Dexaran]

@mgistrat paid https://explorer2.callisto.network/tx/0x469c47b6a81fcc6bfd7e0989745d2aecc38782a072068037e99dfd9e14a1c89c