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: Workaround bug in globwalk. #283

Closed
wants to merge 1 commit into from
Closed

Conversation

wxsBSD
Copy link
Contributor

@wxsBSD wxsBSD commented Jan 9, 2025

As identified in #280 there is a bug in globwalk when using relative paths. Fix it by trying to canonicalize the path, and if that fails falls back to using the relative path. This is fine because the reason canonicalization failed is because the relative path was to a place that doesn't exist, and that does not trigger the bug (it errors out before we get there). We intentionally want this behavior so we get the nicer error reporting with colors. ;)

Fixes #280.

As identified in VirusTotal#280 there is a bug in globwalk when using relative paths. Fix
it by trying to canonicalize the path, and if that fails falls back to using the
relative path. This is fine because the reason canonicalization failed is
because the relative path was to a place that doesn't exist, and that does not
trigger the bug (it errors out before we get there). We intentionally want this
behavior so we get the nicer error reporting with colors. ;)

Fixes VirusTotal#280.
@wxsBSD wxsBSD changed the title Workaround bug in globwalk. fix: Workaround bug in globwalk. Jan 9, 2025
@plusvic
Copy link
Member

plusvic commented Jan 13, 2025

This change has an undesired side-effect. When you pass a relative path to the scan command (like in yr scan my_rule.yar .), the paths of the matching files should be relative to that path (like ./foo/bar), however, after this change all the paths reported by the CLI are absolute paths. That's specially problematic in Windows, where the absolute paths contain drive letters and the \\?\ prefix. See #212.

@plusvic
Copy link
Member

plusvic commented Jan 17, 2025

Fixed in bccf19e with a different approach.

@plusvic plusvic closed this Jan 17, 2025
@wxsBSD wxsBSD deleted the fix_280 branch January 18, 2025 03:17
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 with .\ directory prefix
2 participants