Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas: Improve validateWeights uniqueness check #226

Closed
code423n4 opened this issue Sep 22, 2021 · 2 comments
Closed

Gas: Improve validateWeights uniqueness check #226

code423n4 opened this issue Sep 22, 2021 · 2 comments
Labels
bug Warden finding duplicate Another warden found this issue G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

cmichel

Vulnerability details

The Basket.validateWeights function checks the uniqueness of the tokens by iterating over all tokens for each token.
This runs in O(_tokens.length^2) and is very inefficient as the number of tokens increases.

Recommended Mitigation Steps

Sort the tokens off-chain and provide them already in a sorted (ascending) way.
Then the validateWeights function only needs to verify that the tokens are indeed strictly sorted which runs in linear time:

for (uint i = 0; i < length; i++) {
    require(_tokens[i] != address(0));
    require(_weights[i] > 0);

    // check sorted to ensure uniqueness
    if (i > 0) {
        require(_tokens[i] > _tokens[i - 1]);
    }
}
code423n4 added a commit that referenced this issue Sep 22, 2021
@frank-beard frank-beard added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Oct 19, 2021
@GalloDaSballo
Copy link
Collaborator

Agree with the findings, but want to point that the suggested improvement requires full trust of the input, which may not be a good idea

@GalloDaSballo
Copy link
Collaborator

Duplicate of #160

@GalloDaSballo GalloDaSballo marked this as a duplicate of #160 Nov 26, 2021
@GalloDaSballo GalloDaSballo added the duplicate Another warden found this issue label Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Warden finding duplicate Another warden found this issue G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants