code-423n4 / 2022-02-skale-findings

0 stars 0 forks source link

Missing Signature Verification Leads To Critical Parameter Changes #68

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/CommunityLocker.sol#L209

Vulnerability details

Impact

During the code review, It has been observed that, signature verification is commented out in the protocol. Without off-chain signature verification, an attacker is able to edit parameters in the protocol

Proof of Concept

  1. Navigate to the following contract.

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/CommunityLocker.sol#L209

  1. Signature verification is commented out. The following variables can be edited by an attacker.
        mainnetGasPrice = gasPrice;
        gasPriceTimestamp = timestamp;

Tools Used

Code Review

Recommended Mitigation Steps

Consider uncomment related section. TO-DO should be fixed before the deployment. Ensure that signature verification is enabled before deploy.

cstrangedk commented 2 years ago

Duplicate of #21 and disputed

There is no computation that currently uses mainnetGas price, if you search all contracts you will find no other reference to use this variable. This is a TODO stub, which will be finished and implemented when the Oracle is complete - at which the BLS threshold signature from the nodes will be required to be verified to set the MainnetGasPrice.

GalloDaSballo commented 2 years ago

Agree with sponsor here as the function is unused