code-423n4 / 2023-06-stader-findings

1 stars 1 forks source link

When deploying contracts in PermissionedNodeRegistry.deployWithdrawVault(), PermissionlessNodeRegistry.deployWithdrawVault(), an attacker can find out in advance the address of the future deployed contract and deploy his own at this address #364

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/PermissionlessNodeRegistry.sol#L142-L147 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/PermissionedNodeRegistry.sol#L157-L162 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/factory/VaultFactory.sol#L34-L46

Vulnerability details

Impact

The address of the new contract depends solely on the _salt parameter, which is calculated from user-provided data. Once a user's create transaction is broadcast, the parameters for calculating _salt can be viewed by anyone viewing the public mempool. This will lead to the fact that the attacker will be able to manipulate the distribution of rewards.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider making the upcoming pool address a specific user by concatenating the salt value with the user's address. bytes32 salt = sha256(abi.encode(_poolId, _operatorId, _validatorCount, msg.sender));

Assessed type

Governance

Picodes commented 1 year ago

Exactly the same issue as #370. Please do not try to game the system.

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid