improved the reliability #19
Open
Chintanpatel24 wants to merge 8 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to improve subdomain takeover detection reliability by making DNS record and fingerprint/body matching case-insensitive and normalized, while expanding test coverage and refreshing the project README.
Changes:
- Refactors DNS record matching and HTTP body fingerprint matching in
SubdomainHijackvia centralized helper methods. - Initializes
Fingerprint.FingerprintTextsto an empty list by default to avoid null handling. - Adds matcher-focused unit tests and exposes internals to the test assembly; updates README formatting/wording.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Subdominator/SubdomainHijack.cs | Introduces normalized/case-insensitive matching helpers for DNS and body fingerprints; updates matching call sites. |
| Subdominator/Properties/Assemblyinfo.cs | Adds InternalsVisibleTo to allow tests to call internal matcher helpers. |
| Subdominator/Models/Fingerprint.cs | Initializes FingerprintTexts to new() to avoid null lists. |
| Subdominator.Tests/MatcherTests.cs | Adds unit tests for the new matching helpers (case-insensitivity + normalization). |
| Subdominator.Tests/FingerprintTests.cs | Minor formatting-only change in tests. |
| ReadMe.md | Updates README layout and wording; removes/changes some symbols. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if (normalizedFingerprint.Contains('.')) | ||
| { |
|
|
||
| private static bool LooksLikeIpPrefix(string value) | ||
| { | ||
| return value.All(character => char.IsDigit(character) || character is '.' or ':' or 'x' or 'X'); |
| using System.Collections.Concurrent; | ||
| using System.Text; | ||
| using System.Text.Json; | ||
| using System.Net; |
Comment on lines
+77
to
81
| Finally, if a domain is vulnerable and passes validation with the --validation flag, it will be prepended with a . | ||
| These domains have been validated to be vulnerable with the services directly, not just the fingerprint. For example: | ||
| ``` | ||
| ✅ [Microsoft Azure] example.stratussecurity.com - CNAME: stratus-cdn-stg.azureedge.net | ||
| [Microsoft Azure] example.stratussecurity.com - CNAME: stratus-cdn-stg.azureedge.net | ||
| ``` |
| } | ||
|
|
||
| [TestMethod] | ||
|
|
Comment on lines
+9
to
+10
| Assert.IsTrue(Subdominator.SubdomainHijack.MatchesFingerprintValue("Foo.Example.COM.", "example.com")); | ||
| Assert.IsTrue(Subdominator.SubdomainHijack.MatchesFingerprintValue("151.101.12.34", "151.101.")); |
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.
This pull request introduces several improvements across the codebase and documentation. The most significant changes include making fingerprint and DNS matching logic more robust and case-insensitive, adding new unit tests, and updating the documentation for clarity and consistency.
Enhancements to fingerprint and DNS matching:
SubdomainHijackto ensure DNS value and fingerprint comparisons are case-insensitive and normalized, improving detection accuracy. Added internal static methodsMatchesFingerprintValueandContainsFingerprintTextfor consistent matching across the codebase. [1] [2] [3]FingerprintTextsproperty inFingerprint.csto always be initialized with an empty list, preventing potential null reference issues.Testing improvements:
MatcherTeststo verify that fingerprint matching is case-insensitive and DNS values are properly normalized. Also tests that body fingerprint matching ignores case.InternalsVisibleTo, allowing for more comprehensive internal testing.Documentation updates:
ReadMe.mdby adding a stylized header, removing most emojis for a cleaner look, and making instructions more concise and readable. [1] [2] [3] [4] [5]FingerprintTests.csfor consistency.Fingerprint.csfor consistency.SubdomainHijack.csfor consistency.These changes collectively improve the reliability of subdomain takeover detection, enhance testability, and provide clearer documentation for users and contributors.