From 534afe13abd4960c396d1755defb84ed56fd2306 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 08:53:27 +0000 Subject: [PATCH] fix(sub): treat external subscription providers as untrusted input A client's external subscription is fetched from a third party on every subscriber refresh, and the fetcher trusted it more than its siblings: - it dialled any address, redirects included; it now goes through the SSRF guard like the outbound subscription and remote routing fetchers; - it forwarded any line containing "://"; it now keeps only server share links, so app deeplinks and plain URLs never reach the subscriber's app; - it bounded the body but not the link count (now 1000) or the headers (now 64 KiB), and stored the raw error, URL included, on every row; the stored error is now host-only and at most 1 KiB. The link parser also refuses links that nest JSON past 16 levels, since the JSON subscription pretty-prints it at quadratic cost, and external configs are marshalled compactly until the final, single indent pass. --- internal/sub/external_config.go | 3 +- internal/sub/external_config_test.go | 13 ++++ internal/sub/external_hwid_test.go | 1 + internal/sub/external_subscription.go | 46 ++++++++--- internal/sub/external_subscription_test.go | 78 +++++++++++++++++++ internal/sub/json_service.go | 2 +- internal/util/common/format.go | 21 ++++- internal/util/link/outbound.go | 28 +++++++ internal/util/link/outbound_test.go | 14 ++++ internal/web/service/outbound_subscription.go | 18 ++--- 10 files changed, 203 insertions(+), 21 deletions(-) diff --git a/internal/sub/external_config.go b/internal/sub/external_config.go index ed60be12f..165d5a083 100644 --- a/internal/sub/external_config.go +++ b/internal/sub/external_config.go @@ -203,7 +203,8 @@ func parsedExternalOutbound(rawLink string) json_util.RawMessage { return nil } ob["tag"] = "proxy" - b, err := json.MarshalIndent(ob, "", " ") + // Compact: the response is indented once at the end, and each extra pass costs per level. + b, err := json.Marshal(ob) if err != nil { return nil } diff --git a/internal/sub/external_config_test.go b/internal/sub/external_config_test.go index e41cde482..c3f8d6321 100644 --- a/internal/sub/external_config_test.go +++ b/internal/sub/external_config_test.go @@ -3,6 +3,7 @@ package sub import ( "encoding/base64" "net/url" + "slices" "strings" "testing" "time" @@ -176,3 +177,15 @@ func TestClashProxyFromExternalTrojanReality(t *testing.T) { t.Fatalf("expected reality→tls true, got %v", proxy["tls"]) } } + +// Only server links pass: an app deeplink or a plain URL from the provider would reach the +// subscriber's app as a directive (a routing profile, an import) the operator never chose. +func TestDecodeSubscriptionBodyKeepsOnlyServerShareLinks(t *testing.T) { + body := []byte("VLESS://u@a:1#a\nincy://autorouting/onadd/https://p.example/r.json\nhttps://p.example/page\n" + + "happ://routing/add/x\nvpn://payload\ntuic://u:p@a:1#t\nhy2://p@a:1#h\nnot a link://x\n") + got := decodeSubscriptionBody(body) + want := []string{"VLESS://u@a:1#a", "vpn://payload", "tuic://u:p@a:1#t", "hy2://p@a:1#h"} + if !slices.Equal(got, want) { + t.Fatalf("decoded links = %q, want %q", got, want) + } +} diff --git a/internal/sub/external_hwid_test.go b/internal/sub/external_hwid_test.go index ff39b1a95..a19f89285 100644 --- a/internal/sub/external_hwid_test.go +++ b/internal/sub/external_hwid_test.go @@ -13,6 +13,7 @@ import ( // #6559: the Master panel must send a stable X-HWID when fetching external // subscriptions, otherwise an HWID-limited donor answers 404. func TestFetchSendsStableHwid(t *testing.T) { + allowLoopbackProvider(t) dbtest.InitDB(t, filepath.Join(t.TempDir(), "x-ui.db")) var gotHwid string diff --git a/internal/sub/external_subscription.go b/internal/sub/external_subscription.go index f775fb6b2..adf7f4a21 100644 --- a/internal/sub/external_subscription.go +++ b/internal/sub/external_subscription.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "io" "net/http" + "slices" "strings" "sync" "time" @@ -12,6 +13,8 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/database" "github.com/mhsanaei/3x-ui/v3/internal/database/model" "github.com/mhsanaei/3x-ui/v3/internal/logger" + "github.com/mhsanaei/3x-ui/v3/internal/util/common" + "github.com/mhsanaei/3x-ui/v3/internal/util/netsafe" "github.com/mhsanaei/3x-ui/v3/internal/web/service" ) @@ -21,10 +24,21 @@ import ( const ( subscriptionCacheTTL = 5 * time.Minute subscriptionMaxBytes = 2 << 20 // 2 MiB + subscriptionMaxLinks = 1000 subscriptionCacheCapacity = 256 ) -var subscriptionHTTPClient = &http.Client{Timeout: 6 * time.Second} +// The provider picks every redirect target, so each hop is resolved, checked and dialed by +// the SSRF guard; its headers are bounded like its body. +var subscriptionHTTPClient = &http.Client{ + Timeout: 6 * time.Second, + Transport: &http.Transport{ + DialContext: netsafe.SSRFGuardedDialContext, + TLSHandshakeTimeout: 5 * time.Second, + MaxResponseHeaderBytes: 64 << 10, + IdleConnTimeout: 90 * time.Second, + }, +} type subscriptionCacheEntry struct { links []string @@ -129,7 +143,7 @@ func recordExternalSubscriptionFetch(rawURL string, fetchErr error) { } lastFetchError := "" if fetchErr != nil { - lastFetchError = fetchErr.Error() + lastFetchError = common.TruncateStatusText(service.WithoutRequestURL(fetchErr).Error()) } if err := database.GetDB(). Model(&model.ClientExternalLink{}). @@ -138,14 +152,14 @@ func recordExternalSubscriptionFetch(rawURL string, fetchErr error) { "last_fetch_at": time.Now().UnixMilli(), "last_fetch_error": lastFetchError, }).Error; err != nil { - logger.Warningf("sub: recording fetch status for external subscription %q: %v", rawURL, err) + logger.Warningf("sub: recording fetch status for an external subscription: %v", err) } } func doFetchSubscriptionLinks(rawURL string) ([]string, error) { req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, rawURL, nil) if err != nil { - return nil, err + return nil, service.WithoutRequestURL(err) } // Some providers gate the link body on a known client User-Agent. req.Header.Set("User-Agent", externalSubUserAgent()) @@ -155,7 +169,7 @@ func doFetchSubscriptionLinks(rawURL string) ([]string, error) { } resp, err := subscriptionHTTPClient.Do(req) if err != nil { - return nil, err + return nil, service.WithoutRequestURL(err) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { @@ -168,12 +182,17 @@ func doFetchSubscriptionLinks(rawURL string) ([]string, error) { if len(body) > subscriptionMaxBytes { return nil, errSubscriptionBodyTooLarge } - return decodeSubscriptionBody(body), nil + links := decodeSubscriptionBody(body) + if len(links) > subscriptionMaxLinks { + return nil, errSubscriptionTooManyLinks + } + return links, nil } var ( errBadStatus = &subError{"non-2xx subscription response"} errSubscriptionBodyTooLarge = &subError{"subscription response body exceeds size limit"} + errSubscriptionTooManyLinks = &subError{"subscription lists more than 1000 share links"} ) // externalSubUserAgent returns the panel setting for external subscription @@ -193,8 +212,16 @@ type subError struct{ msg string } func (e *subError) Error() string { return e.msg } -// decodeSubscriptionBody handles the common base64-encoded newline list as well -// as a plain-text body, returning only the lines that look like share links. +// serverShareLinkSchemes are the server links a raw subscription carries on: what the link +// parser reads, what the panel itself writes, and common third-party server schemes. +var serverShareLinkSchemes = []string{ + "vmess", "vless", "trojan", "ss", "ssr", "hysteria", "hysteria2", "hy2", "tuic", "wireguard", "wg", + "vpn", "socks", "socks5", "anytls", "juicity", "naive+https", "tg", +} + +// decodeSubscriptionBody handles the common base64-encoded newline list as well as a +// plain-text body, keeping only server share links: app deeplinks and plain URLs would +// reach the subscriber's app as directives the operator never chose. func decodeSubscriptionBody(body []byte) []string { text := strings.TrimSpace(string(body)) if text == "" { @@ -210,7 +237,8 @@ func decodeSubscriptionBody(body []byte) []string { if ln == "" || strings.HasPrefix(ln, "#") { continue } - if strings.Contains(ln, "://") { + scheme, _, found := strings.Cut(ln, "://") + if found && slices.ContainsFunc(serverShareLinkSchemes, func(s string) bool { return strings.EqualFold(s, scheme) }) { out = append(out, ln) } } diff --git a/internal/sub/external_subscription_test.go b/internal/sub/external_subscription_test.go index 1b0508d1e..e1565a37d 100644 --- a/internal/sub/external_subscription_test.go +++ b/internal/sub/external_subscription_test.go @@ -2,6 +2,7 @@ package sub import ( "errors" + "net" "net/http" "net/http/httptest" "path/filepath" @@ -17,6 +18,17 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/database/model" ) +// allowLoopbackProvider lets a test serve its provider from httptest on 127.0.0.1, which +// the guarded production dialer refuses; every other transport limit stays. +func allowLoopbackProvider(t *testing.T) { + t.Helper() + previous := subscriptionHTTPClient + transport := previous.Transport.(*http.Transport).Clone() + transport.DialContext = (&net.Dialer{}).DialContext + subscriptionHTTPClient = &http.Client{Timeout: previous.Timeout, Transport: transport} + t.Cleanup(func() { subscriptionHTTPClient = previous }) +} + func resetSubscriptionCache(t *testing.T) { t.Helper() subscriptionCache.Lock() @@ -34,6 +46,7 @@ func resetSubscriptionCache(t *testing.T) { } func TestFetchSubscriptionLinksSharesConcurrentRefresh(t *testing.T) { + allowLoopbackProvider(t) resetSubscriptionCache(t) var requests atomic.Int32 release := make(chan struct{}) @@ -69,6 +82,7 @@ func TestFetchSubscriptionLinksSharesConcurrentRefresh(t *testing.T) { } func TestFetchSubscriptionLinksBoundsCacheSize(t *testing.T) { + allowLoopbackProvider(t) resetSubscriptionCache(t) var requests atomic.Int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -96,6 +110,7 @@ func TestFetchSubscriptionLinksBoundsCacheSize(t *testing.T) { } func TestFetchSubscriptionLinksSharesStaleResultAfterRefreshFailure(t *testing.T) { + allowLoopbackProvider(t) resetSubscriptionCache(t) stale := []string{"vless://stale@example.com:443"} release := make(chan struct{}) @@ -150,6 +165,7 @@ func TestFetchSubscriptionLinksSharesStaleResultAfterRefreshFailure(t *testing.T } func TestDoFetchSubscriptionLinks_RejectsOversizedBody(t *testing.T) { + allowLoopbackProvider(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(strings.Repeat("a", subscriptionMaxBytes+1))) })) @@ -165,6 +181,7 @@ func TestDoFetchSubscriptionLinks_RejectsOversizedBody(t *testing.T) { } func TestDoFetchSubscriptionLinks_AcceptsBodyAtLimit(t *testing.T) { + allowLoopbackProvider(t) link := "vless://example" body := link + "\n" + strings.Repeat("#", subscriptionMaxBytes-len(link)-1) if len(body) != subscriptionMaxBytes { @@ -185,6 +202,7 @@ func TestDoFetchSubscriptionLinks_AcceptsBodyAtLimit(t *testing.T) { } func TestRecordExternalSubscriptionFetchStampsEveryRowForTheURL(t *testing.T) { + allowLoopbackProvider(t) initMutDB(t) resetSubscriptionCache(t) db := database.GetDB() @@ -302,6 +320,7 @@ func TestExpandEntryCacheHitWritesNothing(t *testing.T) { } func TestFetchUsesConfiguredExternalSubUserAgent(t *testing.T) { + allowLoopbackProvider(t) resetSubscriptionCache(t) dbtest.InitDB(t, filepath.Join(t.TempDir(), "ua.db")) @@ -330,6 +349,7 @@ func TestFetchUsesConfiguredExternalSubUserAgent(t *testing.T) { } func TestFetchFallsBackToDefaultExternalSubUserAgent(t *testing.T) { + allowLoopbackProvider(t) resetSubscriptionCache(t) dbtest.InitDB(t, filepath.Join(t.TempDir(), "ua-default.db")) @@ -348,3 +368,61 @@ func TestFetchFallsBackToDefaultExternalSubUserAgent(t *testing.T) { t.Fatalf("User-Agent = %q, want default v2rayNG/1.8.5", gotUA) } } + +// The provider chooses where the fetch goes, every redirect included, so the dial itself +// must refuse this host and its network. +func TestDoFetchSubscriptionLinksRefusesPrivateTargets(t *testing.T) { + var hits atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + _, _ = w.Write([]byte("vless://uuid@example.com:443#internal")) + })) + defer srv.Close() + if links, err := doFetchSubscriptionLinks(srv.URL + "/admin"); err == nil || links != nil { + t.Fatalf("fetch of a loopback target = %v, %v; want it refused", links, err) + } + if hits.Load() != 0 { + t.Fatalf("the loopback target received %d request(s)", hits.Load()) + } +} + +// Fetch errors are stored on every row holding the URL and shown in the panel: they may +// name the host, never the URL (a credential), and a provider must not size them. +func TestExternalSubscriptionFetchErrorIsBoundedAndKeepsTheURLSecret(t *testing.T) { + initMutDB(t) + resetSubscriptionCache(t) + allowLoopbackProvider(t) + const token = "s3cr3t-token" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Junk", strings.Repeat("j", 128<<10)) + _, _ = w.Write([]byte("vless://uuid@example.com:443#Node")) + })) + defer srv.Close() + rawURL := srv.URL + "/sub/" + token + if err := database.GetDB().Create(&model.ClientExternalLink{Kind: model.ExternalLinkKindSubscription, Value: rawURL}).Error; err != nil { + t.Fatalf("seed link: %v", err) + } + result := fetchSubscriptionLinks(rawURL) + if result.err == nil { + t.Fatal("a provider with 128 KiB of headers was accepted") + } + recordExternalSubscriptionFetch(rawURL, result.err) + var row model.ClientExternalLink + if err := database.GetDB().Where("value = ?", rawURL).First(&row).Error; err != nil { + t.Fatalf("load link: %v", err) + } + if row.LastFetchError == "" || strings.Contains(row.LastFetchError, token) || len(row.LastFetchError) > 1100 { + t.Fatalf("last_fetch_error = %.200q (%d bytes), want a short error without the URL", row.LastFetchError, len(row.LastFetchError)) + } +} + +func TestDoFetchSubscriptionLinksRefusesTooManyLinks(t *testing.T) { + allowLoopbackProvider(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(strings.Repeat("trojan://p@a:1\n", subscriptionMaxLinks+1))) + })) + defer srv.Close() + if links, err := doFetchSubscriptionLinks(srv.URL); !errors.Is(err, errSubscriptionTooManyLinks) || links != nil { + t.Fatalf("fetch = %d links, %v; want errSubscriptionTooManyLinks", len(links), err) + } +} diff --git a/internal/sub/json_service.go b/internal/sub/json_service.go index 7ea81c84a..c17edb2fa 100644 --- a/internal/sub/json_service.go +++ b/internal/sub/json_service.go @@ -268,7 +268,7 @@ func (s *SubJsonService) GetJson(subId string, host string, alwaysReturnArray bo maps.Copy(newConfigJson, s.bakedTemplate()) newConfigJson["outbounds"] = newOutbounds newConfigJson["remarks"] = remark - newConfig, _ := json.MarshalIndent(newConfigJson, "", " ") + newConfig, _ := json.Marshal(newConfigJson) configArray = append(configArray, newConfig) } } diff --git a/internal/util/common/format.go b/internal/util/common/format.go index f75c90bf8..b290c60b6 100644 --- a/internal/util/common/format.go +++ b/internal/util/common/format.go @@ -1,6 +1,9 @@ package common -import "strconv" +import ( + "strconv" + "unicode/utf8" +) // FormatTraffic formats traffic bytes into human-readable units (B, KB, MB, GB, TB, PB). func FormatTraffic(trafficBytes int64) string { @@ -14,3 +17,19 @@ func FormatTraffic(trafficBytes int64) string { } return strconv.FormatFloat(size, 'f', 2, 64) + units[unitIndex] } + +// maxStatusTextBytes bounds an error text a third party shapes before it is stored as a +// row's status; legitimate messages are a line or two. +const maxStatusTextBytes = 1024 + +// TruncateStatusText cuts s to maxStatusTextBytes on a UTF-8 boundary. +func TruncateStatusText(s string) string { + if len(s) <= maxStatusTextBytes { + return s + } + cut := maxStatusTextBytes + for cut > 0 && !utf8.RuneStart(s[cut]) { + cut-- + } + return s[:cut] + "... (truncated)" +} diff --git a/internal/util/link/outbound.go b/internal/util/link/outbound.go index 98c924fb8..cda31b192 100644 --- a/internal/util/link/outbound.go +++ b/internal/util/link/outbound.go @@ -131,9 +131,37 @@ func ParseLink(link string) (*ParseResult, error) { return nil, err } dropHostLocalKeys(res.Outbound) + if !nestsWithin(map[string]any(res.Outbound), maxOutboundDepth) { + return nil, fmt.Errorf("link nests JSON deeper than %d levels", maxOutboundDepth) + } return res, nil } +// maxOutboundDepth bounds the JSON a link can nest into its outbound (the deepest real shape, +// an xhttp download stream's finalmask, is about 11): pretty-printing costs depth squared. +const maxOutboundDepth = 16 + +func nestsWithin(v any, depth int) bool { + if depth < 0 { + return false + } + switch v := v.(type) { + case map[string]any: + for _, child := range v { + if !nestsWithin(child, depth-1) { + return false + } + } + case []any: + for _, child := range v { + if !nestsWithin(child, depth-1) { + return false + } + } + } + return true +} + func parseLinkByScheme(link string) (*ParseResult, error) { switch { case strings.HasPrefix(link, "vmess://"): diff --git a/internal/util/link/outbound_test.go b/internal/util/link/outbound_test.go index d93fa943b..4bf101f12 100644 --- a/internal/util/link/outbound_test.go +++ b/internal/util/link/outbound_test.go @@ -661,3 +661,17 @@ func TestParseLinkDropsHostLocalKeys(t *testing.T) { t.Fatalf("remote-server settings were dropped too: %s", raw) } } + +// Pretty-printing costs the square of the nesting depth, and extra=, fm= and the vmess +// JSON copy provider JSON into the outbound; a link may not nest past real shapes. +func TestParseLinkRejectsDeeplyNestedJSON(t *testing.T) { + deep := strings.Repeat(`{"a":`, 40) + `1` + strings.Repeat(`}`, 40) + if _, err := ParseLink("vless://uuid@8.8.8.8:443?type=xhttp&security=tls&extra=" + url.QueryEscape(`{"xmux":`+deep+`}`) + "#n"); err == nil { + t.Fatal("a link nesting JSON 40 levels deep was accepted") + } + real := url.QueryEscape(`{"downloadSettings":{"address":"8.8.8.8","port":443,"network":"xhttp","xhttpSettings":{"headers":{"a":"b"}},` + + `"finalmask":{"udp":[{"type":"noise","settings":{"noise":[{"type":"rand","packet":"10-20"}]}}]}}}`) + if _, err := ParseLink("vless://uuid@8.8.8.8:443?type=xhttp&security=tls&extra=" + real + "#n"); err != nil { + t.Fatalf("a real download stream with a finalmask was refused: %v", err) + } +} diff --git a/internal/web/service/outbound_subscription.go b/internal/web/service/outbound_subscription.go index 99973ca1a..759847370 100644 --- a/internal/web/service/outbound_subscription.go +++ b/internal/web/service/outbound_subscription.go @@ -172,7 +172,7 @@ func (s *OutboundSubscriptionService) nextDefaultSubPrefix(excludeId int) (strin func (s *OutboundSubscriptionService) Create(remark, rawURL, tagPrefix, userAgent string, enabled bool, updateInterval int, allowPrivate, prepend, allowInsecure bool) (*model.OutboundSubscription, error) { cleanURL, err := SanitizePublicHTTPURL(rawURL, allowPrivate) if err != nil { - return nil, common.NewError("invalid subscription URL:", withoutRequestURL(err)) + return nil, common.NewError("invalid subscription URL:", WithoutRequestURL(err)) } if cleanURL == "" { return nil, common.NewError("subscription URL is required") @@ -224,7 +224,7 @@ func (s *OutboundSubscriptionService) Update(id int, remark, rawURL, tagPrefix, } cleanURL, err := SanitizePublicHTTPURL(rawURL, allowPrivate) if err != nil { - return common.NewError("invalid subscription URL:", withoutRequestURL(err)) + return common.NewError("invalid subscription URL:", WithoutRequestURL(err)) } if cleanURL == "" { return common.NewError("subscription URL is required") @@ -368,7 +368,7 @@ func (s *OutboundSubscriptionService) fetchAndStore(sub *model.OutboundSubscript "last_fetched_outbounds": string(obsJSON), "link_identities": fetched.identities, "last_updated": time.Now().Unix(), - "last_error": fetched.lastError, + "last_error": common.TruncateStatusText(fetched.lastError), }) if res.Error != nil { return nil, res.Error @@ -401,7 +401,7 @@ func (s *OutboundSubscriptionService) fetchOutbounds(sub *model.OutboundSubscrip // subscription was explicitly created with AllowPrivate. cleanURL, err := SanitizePublicHTTPURL(sub.Url, sub.AllowPrivate) if err != nil { - return nil, withoutRequestURL(err) + return nil, WithoutRequestURL(err) } if cleanURL == "" { return nil, common.NewError("subscription has no valid URL") @@ -429,7 +429,7 @@ func (s *OutboundSubscriptionService) fetchOutbounds(sub *model.OutboundSubscrip reqCtx := netsafe.ContextWithAllowPrivate(context.Background(), sub.AllowPrivate) req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, cleanURL, nil) if err != nil { - return nil, withoutRequestURL(err) + return nil, WithoutRequestURL(err) } userAgent := strings.TrimSpace(sub.UserAgent) if userAgent == "" { @@ -443,7 +443,7 @@ func (s *OutboundSubscriptionService) fetchOutbounds(sub *model.OutboundSubscrip resp, err := client.Do(req) if err != nil { - return nil, withoutRequestURL(err) + return nil, WithoutRequestURL(err) } defer resp.Body.Close() @@ -557,9 +557,9 @@ func spendXMCBudget(outbound any, budget *int) bool { return true } -// withoutRequestURL keeps only the host of the URL net/url and net/http quote in their +// WithoutRequestURL keeps only the host of the URL net/url and net/http quote in their // errors: a subscription URL is a credential, and these errors are logged and stored. -func withoutRequestURL(err error) error { +func WithoutRequestURL(err error) error { var urlErr *url.Error if !errors.As(err, &urlErr) { return err @@ -588,7 +588,7 @@ func forceUserspaceWireguard(outbound any) { } func (s *OutboundSubscriptionService) recordError(sub *model.OutboundSubscription, err error) { - sub.LastError = err.Error() + sub.LastError = common.TruncateStatusText(err.Error()) _ = database.GetDB().Model(sub).Update("last_error", sub.LastError).Error }