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 slicing panic in path completion variable expansion #12556

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

the-mikedavis
Copy link
Member

The regexes in expand_impl may match multiple times on a single variable like ${HOME:-$HOME}: once for the ${HOME:- part and again for its default value $HOME. This could cause a panic because of an invalid slice &bytes[pos..range.start]: pos is set to range.end which corresponds to the closing } in the first iteration of the loop, so it's greater than range.start in the second iteration.

Fixes #12552

@the-mikedavis the-mikedavis added this to the 25.01.1 milestone Jan 16, 2025
@pascalkuthe
Copy link
Member

pascalkuthe commented Jan 16, 2025

Hmm usually regex matches should be non-overlapping. There is an overlapping mode but AFAIK not enabled

@pascalkuthe
Copy link
Member

pascalkuthe commented Jan 16, 2025

Ah I think I get it now, I am tired. Could you add the example to the tests rest lgtm

@the-mikedavis
Copy link
Member Author

Yeah exactly, in this case r"\$\{([^\}:]+):-" and r"\$(\w+)" don't actually overlap

The regexes in `expand_impl` may match multiple times on a single
variable like `${HOME:-$HOME}`: once for the `${HOME:-` part and
again for its default value `$HOME`. This could cause a panic because
of an invalid slice `&bytes[pos..range.start]`: `pos` is set to
`range.end` which corresponds to the closing `}` in the first iteration
of the loop, so it's greater than `range.start` in the second iteration.
@the-mikedavis the-mikedavis merged commit ffdfb59 into master Jan 17, 2025
6 checks passed
@the-mikedavis the-mikedavis deleted the md-path-expand-fix branch January 17, 2025 00:10
mmibbetson pushed a commit to mmibbetson/helix that referenced this pull request Jan 18, 2025
nik-rev pushed a commit to nik-rev/helix that referenced this pull request Jan 24, 2025
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.

Panic in expand_impl due to invalid slice indices in helix-stdx/src/env.rs
3 participants