The function `update_agents()` doesn't check for the new weight to be bigger than the older weight of the agent

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xed5bead296eaa4dff3df077c5706ace63e92d213c3e27ea9a282d67606d6dc9b Severity: medium

Description: Description\

In regsitry smart contract, there is a function that updates the agent's weight - update_agents(). The problem is that the function doesn't check for the new weight to be always bigger than the older weight which can lead to a situation where the weight of an agent is lower than the pooled azero.

Attack Scenario\

Let's consider the function update_agents:

https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/blob/main/src/registry/lib.rs#L180-181:

  self.total_weight -= old_weight;
  self.total_weight += new_weight;

So if the agent's weight is 100 and it's set to 80, for example, it lead to a situation where his weight can be lower than the pooled azero. This will significantly impact the functionality and may lead to tx revert when delegate_unbonding():

let (total_weight, agents) = self.registry_contract.get_agents();

The agent may have the pooled assets = 100 but his weight will be 80.

The similar check is implemented when calling remove_agent() but is missing in update_agents():

https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/blob/main/src/registry/lib.rs#L224-226

  if weight > 0 {
                    return Err(RegistryError::ActiveAgent);
                }

Recommendation

Add the following check:

  if new_weight  < old_weight {
                    return Err(RegistryError::WeightIsLower);
                }

[0xmahdirostami]

Thank you for your submission. owner needs to be able to decrease and increase the weight.

[rodiontr]

Thank you for your submission. owner needs to be able to decrease and increase the weight.

Yes, the owner has to have this ability. But in the remove_agent() (removing is the same as decreasing weight to 0), there is such check that checks whether the agent have pooled assets and if so, he cannot be removed. And it can be a situation where the weight is suddenly decreased to some value that is lower than pooled assets for that agent. I think it's not medium though, it more of a low

[0xmahdirostami]

it can be a situation where the weight is suddenly decreased to some value that is lower than pooled assets for that agent.

the weight is not about the total pooled. the weight is compared to other weights and the total weight to stake more or less in one agent.

scenario:

there are 3 agents with 100 weights (100, 100, 100) and 5m AZERO is pooled in each. the owner decided to pool in two of them more than the third one, so the owner decreased the third agent's weight compared to others 100, 100, 50 now there are 6M in first and second agents and 3M in third agent

[bgibers]

it can be a situation where the weight is suddenly decreased to some value that is lower than pooled assets for that agent.

the weight is not about the total pooled. the weight is compared to other weights and the total weight to stake more or less in one agent.

scenario:

there are 3 agents with 100 weights (100, 100, 100) and 5m AZERO is pooled in each. the owner decided to pool in two of them more than the third one, so the owner decreased the third agent's weight compared to others 100, 100, 50 now there are 6M in first and second agents and 3M in third agent

This is correct, the weight is used to move around staked funds within the pools. We will be constantly retargeting weights to different pools based off performance. See our docs for more details