fix(transcribe): block shorthand IPv4 spellings of internal hosts - #564
Closed
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Closed
fix(transcribe): block shorthand IPv4 spellings of internal hosts#564SEPURI-SAI-KRISHNA wants to merge 1 commit into
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Conversation
Owner
|
感谢发现 shorthand IPv4 SSRF 绕过。你的原始作者提交已保留在 merged #577 中,并扩展覆盖 Unicode IDNA、percent-encoded host 与反斜杠 authority,完整 CI 已通过,因此关闭原 PR。 |
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.
Problem
_assert_safe_public_url()inagent_reach/transcribe.pyis the SSRF guard added by #443 (closing #444). It rejects internal targets by parsing the host withipaddress.ip_address()— which only accepts the canonical dotted-quad form.The C resolver behind
yt-dlpaccepts the wholeinet_atongrammar. So every non-canonical spelling of an internal address walks straight past the check:transcribehttp://127.1/127.0.0.1http://127.0.1/127.0.0.1http://2130706433/127.0.0.1http://0x7f000001/127.0.0.1http://0177.0.0.1/127.0.0.1http://0/0.0.0.0http://192.168.1/192.168.0.1http://2852039166/169.254.169.254http://0xA9FEA9FE/169.254.169.254The last two reach the cloud instance-metadata endpoint — the exact target the guard names in its own
_BLOCKED_HOSTSneighbourhood (metadata.google.internal).http://169.254.169.254/is blocked;http://0xA9FEA9FE/is not.Reproduce on
main:This matters because
transcribe(source)takes its URL fromagent-reach transcribe <source>and fromYouTubeChannel.transcribe(), i.e. from an agent or from scraped content — precisely the untrusted-input path #444 was filed about.Fix
Parse the host with the same grammar the resolver uses, then run the existing private/loopback/link-local/reserved/multicast/unspecified check against the canonicalised address.
socket.inet_atonis the same libc parsergetaddrinfouses for IPv4 literals, and it rejects real hostnames (example.com,localhost,1.2.3.4.5→OSError), so it is a pure literal parse.No DNS lookup is added. The existing
test_does_not_dns_resolve_public_hostnamescontract is preserved, and a new test asserts the shorthand path stays DNS-free by makingsocket.getaddrinforaise.Public literal addresses are unaffected — including
http://010.010.010.010/, which is octal for the public8.8.8.8and stays allowed.Tests
tests/test_transcribe.py, in the existingTestDownloadAudioSafetyclass:test_rejects_shorthand_ipv4_spellings_of_internal_hosts— 10 parametrised spellings, each assertingyt-dlpis never invokedtest_shorthand_ipv4_check_stays_dns_free— fails if the new parsing resolves a nametest_allows_public_literal_addresses— regression guard for1.1.1.1,8.8.8.8,010.010.010.010pytest -q: 442 passed (was 428).ruff checkclean on both touched files.Scope
Deliberately narrow — one function, no behaviour change for any valid input. It does not touch the separate
WebChannelSSRF discussion in #456/#370, and does not add redirect-following or DNS-rebinding protection; those remain out of scope for this guard.