Description

Before aggregating a BLS public key, a proof-of-possession is required by the contract. It consists in a signature of the domain separation tag, the public key together with the caller address, and the service-node public key. This allows proving to the contract that the service node knows the correct private key matching the public key submitted. It means that a node knows the private key $x_i$ of its public key $Q_i$ such that $Q_i=x_i\cdot G_1$, where $G_1$ is the generator of the group ${\mathbb{G}}_1$.

However, the function seedPublicKeyList does not use a proof-of-possession before adding a list of public keys to the aggregate key:

/// @notice Seeds the public key list with an initial set of service nodes.
///
/// @dev This should be called before the hardfork by the foundation to
/// ensure the public key list is ready to operate.
///
/// @param pkX Array of X-coordinates for the public keys.
/// @param pkY Array of Y-coordinates for the public keys.
/// @param amounts Array of amounts that the service node has staked,
/// associated with each public key.
function seedPublicKeyList(
    uint256[] calldata pkX,
    uint256[] calldata pkY,
    uint256[] calldata amounts
) external onlyOwner {
    if (pkX.length != pkY.length || pkX.length != amounts.length) {
        revert ArrayLengthMismatch();
    }

    for (uint256 i = 0; i < pkX.length; i++) {
        BN256G1.G1Point memory pubkey = BN256G1.G1Point(pkX[i], pkY[i]);
        uint64 allocID = serviceNodeAdd(pubkey);
        _serviceNodes[allocID].deposit = amounts[i];
        emit NewSeededServiceNode(allocID, pubkey);
    }

    updateBLSNonSignerThreshold();
}

Therefore, a rogue key attack is possible. For an aggregate key $Q={\sum }_i{Q}_i$, the owner of the contract can add a new rogue key $Q_r=x_r\cdot G-{\sum }_i{Q}_i$ to the aggregate key. Then the new aggregate key verifies signatures signed only by their key $x_r$ but not the other previous keys. This means that messages signed only by their key will be seen as valid even if they were not signed by the other public keys.

Impact

Even if this function is callable only by the owner of the contract, it allows the owner to add a rogue key afterwards and have the signature that is signed only by them seen as valid.

Recommendations

Ensure proof-of-possession before all aggregate key modification.

Remediation

This issue has been acknowledged by Session team, and a fix was implemented in commit 9c97d798↗.

Session team's official response is paraphrased below:

The function seedPublicKeyList reverts if the contract has already started, when the variable isStarted is set to true. The keys passed as parameter before the contract started, will be handled out-of-band via our C++ code that will be responsible for checking the proof-of-possession.