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

fix an out of bound access in keyListResponseWithCorrectSize #649

Merged
merged 2 commits into from
May 12, 2022

Conversation

dhiaayachi
Copy link
Contributor

@dhiaayachi dhiaayachi commented May 11, 2022

Fix #647

The function keyListResponseWithCorrectSize truncate the key list until if fit in q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength.

The logic in keyListResponseWithCorrectSize is flawed, for the special case where the list of keys is smaller then the min required size q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength an out of bound access will happen in here

The fix proposed in here is to loop through the minimum of the actual key size and q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength, this will ensure that:

  • for normal scenarios where q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength is smaller then the number of keys the logic is unchanged
  • for the scenario where the number of keys is smaller then q.serf.config.QueryResponseSizeLimit / minEncodedKeyLength we will start with the full list of key and then try to truncate

@dhiaayachi dhiaayachi requested a review from markan May 12, 2022 17:23
Copy link

@markan markan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

response size check on keyListResponseWithCorrectSize breaks with 32bit keys and keyring size >20
2 participants