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 reading past end of array. #1468

Merged
merged 1 commit into from
Jan 28, 2025
Merged

Conversation

dlech
Copy link
Contributor

@dlech dlech commented Jan 12, 2025

Fix a bug in the NonDelegatingGetIids() implementation that could cause reading past the end of the returned array.

std::copy() returns a pointer to the next element in the array after the last element copied. The output argument *array was being assinged to this pointer after the first copy, causing it to no longer point to the beginning of the array. If the caller tries to access the full array after this, it will read past the end of the array and will miss the first elements of the array.

To fix, introduce a new temporary _array variable to pass the result of the first copy as the starting point of the second copy. Also add a test that failed before the fix and passes after the fix.

@dlech
Copy link
Contributor Author

dlech commented Jan 12, 2025

If we aren't going to merge #1467, this fixes a bug that was noted there.

strings/base_implements.h Outdated Show resolved Hide resolved
test/old_tests/UnitTests/Composable.cpp Outdated Show resolved Hide resolved
Fix a bug in the `NonDelegatingGetIids()` implementation that could
cause reading past the end of the returned array.

`std::copy()` returns a pointer to the next element in the array after
the last element copied. The output argument `*array` was being assinged
to this pointer after the first copy, causing it to no longer point to
the beginning of the array. If the caller tries to access the full array
after this, it will read past the end of the array and will miss the
first elements of the array.

To fix, introduce a new temporary `_array` variable to pass the result
of the first copy as the starting point of the second copy. Also add a
test that failed before the fix and passes after the fix.
@dlech
Copy link
Contributor Author

dlech commented Jan 20, 2025

Updated:

  • Renamed _array to next.
  • Changed test == 5.

@DefaultRyan
Copy link
Member

Looks good to me.

@DefaultRyan DefaultRyan merged commit 7bc7df6 into microsoft:master Jan 28, 2025
75 checks passed
@dlech dlech deleted the fix-array-fill branch January 28, 2025 23:23
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.

3 participants