fix(favicon): read icons through the shared cap and skip non-images - #381
Open
TBX3D wants to merge 3 commits into
Open
fix(favicon): read icons through the shared cap and skip non-images#381TBX3D wants to merge 3 commits into
TBX3D wants to merge 3 commits into
Conversation
…dentically scan.Favicon capped icon reads at 1MB while the module matcher path reads via httpx.ReadCappedBody's 5MB cap, so the two SSOT paths fed different byte streams into the same shared FaviconHash for any icon over 1MB, producing a different hash and a wrong -favicon shodan pivot. drop the standalone faviconBodyReadCap constant and read through httpx.MaxBodySize instead.
getFaviconBytes only rejected a non-200 status or an empty body, so a 200 text/html soft-404 page (common when an app has no real 404 handler) got hashed as if it were the icon, producing a bogus hash and shodan pivot. the doc comment claimed soft-404s were already excluded, which was not true since soft-404s are 200s. now the body must carry an image/* content-type or sniff as a known icon format (ico, png, gif, jpeg, webp, svg) before it's hashed.
modules/info/favicon-gitlab.yaml hardcodes a hash that duplicates fingerprint.faviconTech rather than being generated from it, with nothing binding the two together. add a test that walks modules/**/favicon-*.yaml, extracts every declared favicon hash and checks it exists in the table (and, when the module name mentions a tech, that the name agrees with the table). verified the guard actually fails on both an unknown hash and a mismatched tech name before confirming the current yaml matches the table with no existing drift.
github-actions
Bot
added
size/l
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #381 +/- ##
=======================================
Coverage ? 64.90%
=======================================
Files ? 88
Lines ? 7875
Branches ? 0
=======================================
Hits ? 5111
Misses ? 2371
Partials ? 393 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
pr summary3 files changed (+255 -9)
|
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
2 participants
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.
three problems in the favicon path, all of them producing a hash that is wrong rather than absent.
scan.Favicon capped icon reads at 1MB while the module matcher path reads through httpx.ReadCappedBody at 5MB, so any icon over 1MB fed two different byte streams into the same shared FaviconHash and gave two different shodan pivots. getFaviconBytes also only rejected non-200s and empty bodies, so a 200 text/html soft-404 got hashed as an icon, which the doc comment claimed was already handled. and favicon-gitlab.yaml hardcodes a hash duplicating fingerprint.faviconTech with nothing binding the two, so the new drift test walks favicon-*.yaml and checks every declared hash against the table.