registerLayer2Candidate() function can be abused to register a vulnerable

[suahnkim]

Describe the bug
Community member @blackcow1987 reported an issue with registerLayer2Candidate(), where anyone can register any candidate contract to the DAO. This includes a case where a user's staked TON (for that candidate) can be burned by manipulating who the operator is. @blackcow1987 also submitted test script to show feasibility of the attack.

Configuration
no severity label (won't be fixed)

Impact

• Affected scope: Attacks can be carried out by registering a custom candidate contract with an attack logic, to DAO. The user must stake to that candidate to be affected by the attack.

Please note that this issue has already been reported by at least three members from Tokamak Network prior to this report. The reason for not removing this function is as follows:

• Tokamak Network's commitment for open development: Tokamak Network believes in community development and understands that the platform can grow by not restricting functionality. By facilitating this policy, Tokamak Network recognizes that more attacks could be possible from the contract level. However, any frontend operated by Tokamak Network will not allow any abuse of this function.
• Contract risk is compensated from the frontend: Simple Staking and DAO do not list candidates registered using the reported function (only the candidates registered using createCandidate() are shown on the frontend. Thus, any layer2Candidate codes not vetted by Tokamak Network are not shown on the frontend. Users must stake directly by calling from the contract or develop their own frontend (As of this point, there is no attack that can be implemented that affects stakers on different DAO candidate).

Recommendation
No fix is required and no bounty will be paid.

• However, we will improve the UX for simple staking and DAO by adding a warning text that summarizes this issue. It will explain that candidates registered via this function will not be shown since the code has not been checked.
• Also, please note that since the plasm EVM implementation development is officially deprecated and there are no interest in developing the candidate contract from the community after many years, registerLayer2Candidate() was removed from the current staking V2 code (prior to this report).

Report Contributors

• @ggs134 @usgeeus @zzooppii @Zena-park @suahnkim

This issue will be closed if there are no additional comments by September 2024

[blackcow1987]

@suahnkim
I think the following scenario is possible:

1. The register method is still maintained in ton-staking-v2
   -> https://github.com/tokamak-network/ton-staking-v2/blob/main/contracts/stake/Layer2Registry.sol#L70
2. A malicious user can register a Candiate by calling the registerLayer2Candidate method on the layer2 contract.
   -> https://github.com/tokamak-network/ton-staking-v2/blob/main/contracts/dao/DAOCommitteeExtend.sol#L349C14-L349C37

If the Foundation missed the scenario, I think it should be classified as a new vulnerability, separate from the previously reported ones.

ps. Also, if it is a previously discovered vulnerability, it would be good to provide evidence as the first report.

[suahnkim]

@blackcow1987 hey, sorry for the late response, we have finished discussing internally and will post our answer soon.

[suahnkim]

@suahnkim I think the following scenario is possible:

1. The register method is still maintained in ton-staking-v2
   -> https://github.com/tokamak-network/ton-staking-v2/blob/main/contracts/stake/Layer2Registry.sol#L70
2. A malicious user can register a Candiate by calling the registerLayer2Candidate method on the layer2 contract.
   -> https://github.com/tokamak-network/ton-staking-v2/blob/main/contracts/dao/DAOCommitteeExtend.sol#L349C14-L349C37

If the Foundation missed the scenario, I think it should be classified as a new vulnerability, separate from the previously reported ones.

ps. Also, if it is a previously discovered vulnerability, it would be good to provide evidence as the first report.

@blackcow1987 Sorry for the delay response:

A1.
Yes, register() is still open on Layer2Registry, and this function can be called by the MinterOrOperator of the layer2. However, even if register() is called, it does not register them as a DAO candidate. It only registers them on the list of L2 and is not shown on the frontend (because it's not a DAO candidate and wasn't created using createCandidate()). We don't consider this a vulnerability.

A2.
This question has been answered before, but perhaps it wasn't clearly communicated. From Tokamak Network's dev team's perspective, we also agree and recognize that registerLayer2Candidate() is a dangerous call.

To foster a wide variety of development, we decided to leave this open at the contract level. We don't show this candidate on our frontend to prevent novice users from staking on any candidates that weren't created using createCandidate()

This policy is similar to how anyone can create a malicious contract that uses an existing UniswapV3 contract's swap function and build a frontend for it. Users need to verify that the frontend is correct and the contract is safe to use. If there's any loss from this frontend, it's not Uniswap's responsibility. Limiting the composability of dapps would hinder the development of new dapps.

@blackcow1987 If you are not satisfied with the answer, we would be more than happy to have a call with you and discuss. As always, we appreciate your report and feedback, and sorry for the delay.

If you are ok with the answer, we will close this issue after your confirmation.