Description

Upon examining the code where the _aggregatePubkey is updated through the addBLSPublicKey function, we have identified a potentially significant scenario that requires attention. Assuming that the DEX swap pair that includes SENT tokens has a large liquidity in the future, there exists a potential vulnerability.

Specifically, if a flash loan is used to acquire enough tokens to constitute more than a 2/3 majority, and these tokens are then staked, the _aggregatePubKey could be updated immediately.

The code involved is shown below.

function serviceNodeAdd(BN256G1.G1Point memory pubkey) internal returns (uint64 result) {
    [...]
    if (totalNodes == 1) {
        _aggregatePubkey = pubkey;
    } else {
        _aggregatePubkey = BN256G1.add(_aggregatePubkey, pubkey);
    }
    return result;
}

This scenario could enable arbitrary reward updates using the updateRewardsBalance function.

Impact

If exploited, this vulnerability could allow an attacker to perform arbitrary updates to the rewards balance. This could lead to manipulation of rewards and financial losses.

Recommendations

To mitigate this risk, it would be prudent to implement a minimum waiting period before updating the _aggregatePubKey after the execution of the serviceNodeAdd function, rather than allowing an immediate update. This would prevent the described exploit and ensure greater security and integrity of the system.

While the project team may monitor the difference between the liquidity of the DEX pair and the liquidity of the ServiceNodeRewards contract and take appropriate actions to avoid such scenarios, Session team should also consider the risks that could be prevented by human monitoring.

Remediation

This issue has been acknowledged by Session team, and a fix was implemented in commit 8cbd1fb7↗. It was addressed by adding a limit that allows up to max(5, 2% of totalNodes) public keys to be aggregated into the _aggregatePubkey within a single block.