code-423n4 / 2021-09-sushitrident-findings

0 stars 0 forks source link

Similarly initialized weight thresholds may cause unexpected deployment failures #48

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

MAX_WEIGHT and MAX_TOTAL_WEIGHT are both initialized to the same value of BASE * 50, in IndexPool, which means that a single token of MAX_WEIGHT is allowed but pool creation will fail because the MIN_WEIGHT of any other tokens added will exceed the MAX_TOTAL_WEIGHT for the pool. This will cause unexpected deployment failures if the deployer doesn’t account for such similarly initialized weight thresholds.

Proof of Concept

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/pool/IndexPool.sol#L30-L31

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/pool/IndexPool.sol#L70

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/pool/IndexPool.sol#L76

Tools Used

Manual Analysis

Recommended Mitigation Steps

Change MAX_TOTAL_WEIGHT to a different/higher value and check accordingly to make sure multiple tokens can be accommodated in the Index Pool with appropriate max/min/total weight thresholds.

alcueca commented 2 years ago

Can't find a duplicate, @maxsam4?

maxsam4 commented 2 years ago

Yeah, I can't find it anymore either. In any case, I don't see the issue here. If you assign 100% of the weight to 1 token, you obviously can't assign anything to other tokes so the deployment should and does fail.

alcueca commented 2 years ago

Agree in part with the sponsor, I see this as a code clarity issue. The fact that there is no risk seems to be incidental, and not by design. Unclear in any case.