Fix URL security vulnerabilities in string matching#2
Merged
Conversation
Security issue: String matching for domains and protocols at arbitrary positions in URLs could be exploited with malicious URLs like: - evil.com/git+malicious (matches "git+" check) - evil-github.com-fake.ru (matches "github.com" check) Changes: 1. src/ossval/parsers/spdx.py: - Add _is_git_url() helper method for safe URL validation - Properly parse URLs and check domain via urlparse.netloc - Check protocol prefixes with startswith() instead of 'in' - Fix 3 vulnerable checks in JSON and tag-value parsing 2. src/ossval/analyzers/repo_finder.py: - Fix _is_valid_git_url() to parse URLs and check netloc - Fix _normalize_git_url() to safely check domains - Use urlparse to extract and verify domain names - Check protocol prefixes with startswith() 3. src/ossval/core.py: - Fix GitHub health check to parse URL and validate netloc - Replace unsafe 'in' check with urlparse validation - Only analyze health for actual github.com domain Security improvements: - All domain checks now use urlparse().netloc == "domain.com" - Protocol checks use startswith() instead of substring matching - Prevents bypasses via domain name in path or subdomain - All tests passing (94 tests)
- Update version in pyproject.toml and __init__.py - Add 1.2.2 release notes to CHANGELOG.md - Document security fixes for URL validation vulnerabilities
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security Issue
String matching for domains and protocols at arbitrary positions in URLs could be exploited with malicious URLs:
Examples of vulnerable patterns:
evil.com/git+malicious→ matches"git+" in urlevil-github.com-fake.ru→ matches"github.com" in urlhttps://malicious.com?redirect=github.com/evil→ matches domain checkChanges
1. src/ossval/parsers/spdx.py
_is_git_url()helper method for safe URL validationurlparse().netlocstartswith()instead ofinoperator2. src/ossval/analyzers/repo_finder.py
_is_valid_git_url()to parse URLs and check netloc_normalize_git_url()to safely check domainsstartswith()3. src/ossval/core.py
incheck with urlparse validationSecurity Improvements
✅ All domain checks now use
urlparse().netloc == "domain.com"✅ Protocol checks use
startswith()instead of substring matching✅ Prevents bypasses via domain name in path or subdomain
✅ All tests passing (94 tests)
Testing
pytest tests/ -v # 94 passed, 13 warnings in 8.10sImpact