code-423n4 / 2021-10-slingshot-findings

0 stars 0 forks source link

No Input Validation When Setting Up Values For Immutable State Variables (Slingshot.sol) #89

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

ye0lde

Vulnerability details

Impact

Immutable state variables can't be changed after initialization in the constructor, so their values should be checked before initialization.

Proof of Concept

In Slingshot.sol immutable variables "nativeToken" and "wrappedNativeToken" are set without any checks on the input parameters used to initialize them. https://github.com/code-423n4/2021-10-slingshot/blob/9c0432cca2e43731d5a0ae9c151dacf7835b8719/contracts/Slingshot.sol#L53-L54

Those same unchecked parameters (_nativeToken, _wrappedNativeToken) are also used to create and set "executioner". There is no verification in Executioner.sol of these parameters. https://github.com/code-423n4/2021-10-slingshot/blob/9c0432cca2e43731d5a0ae9c151dacf7835b8719/contracts/Slingshot.sol#L51 https://github.com/code-423n4/2021-10-slingshot/blob/9c0432cca2e43731d5a0ae9c151dacf7835b8719/contracts/Executioner.sol#L26-L27

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Recommend adding a "require" condition to validate input values.

tommyz7 commented 2 years ago

I believe input validation is non-critical. Deployment configuration is double checked and is also tested in test suit (not included in audit).

alcueca commented 2 years ago

Agree with sponsor. Input validation should happen off-chain and for governance related data, on fork tests.