From 2d72d9c7158c9c70df4a3b430e725f488fe75a48 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 08:23:24 +0000 Subject: [PATCH] fix(xray): check outbounds the way the core's handlers will ValidateOutboundConfig ran only conf.Build(), but the core parses some fields again in its handler constructors, and a failure there stops the whole core at start: wireguard local addresses and peers, the vless encryption keys and padding, and shadowsocks 2022 keys. Mirror those parses with the core's own helpers, and turn a panic in the loader into a rejection so one bad link cannot unwind the caller. --- internal/xray/api.go | 97 +++++++++++++++++++++-- internal/xray/outbound_validation_test.go | 64 +++++++++++++++ 2 files changed, 154 insertions(+), 7 deletions(-) diff --git a/internal/xray/api.go b/internal/xray/api.go index 0e0e68ddc..906b348f9 100644 --- a/internal/xray/api.go +++ b/internal/xray/api.go @@ -5,6 +5,7 @@ package xray import ( "context" + "encoding/base64" "encoding/json" "fmt" "math" @@ -36,6 +37,8 @@ import ( "github.com/xtls/xray-core/proxy/shadowsocks_2022" "github.com/xtls/xray-core/proxy/trojan" "github.com/xtls/xray-core/proxy/vless" + vlessEncryption "github.com/xtls/xray-core/proxy/vless/encryption" + vlessOutbound "github.com/xtls/xray-core/proxy/vless/outbound" "github.com/xtls/xray-core/proxy/vmess" wireguard "github.com/xtls/xray-core/proxy/wireguard" "google.golang.org/grpc" @@ -180,8 +183,14 @@ func (x *XrayAPI) DelInbound(tag string) error { // xray-core config loader, surfacing the exact error the core would raise at // startup — notably v26.7.11's refusal of unencrypted vless/trojan outbounds // whose server address is a public IP or domain. -func ValidateOutboundConfig(outbound []byte) error { +func ValidateOutboundConfig(outbound []byte) (err error) { ensureXrayAssetLocation() + // The loader panics on some third-party values (a short vless encryption, a bad spiderX). + defer func() { + if r := recover(); r != nil { + err = common.NewErrorf("xray-core config loader panicked: %v", r) + } + }() detour := new(conf.OutboundDetourConfig) if err := json.Unmarshal(outbound, detour); err != nil { @@ -192,12 +201,12 @@ func ValidateOutboundConfig(outbound []byte) error { if err != nil { return err } - return validateWireguardRemoteDNS(built.ProxySettings) + return validateHandlerSettings(built.ProxySettings) } -// validateWireguardRemoteDNS refuses what conf.Build() lets through but the core feeds to -// netip.MustParseAddr at startup: a non-IP remoteDNS entry, like "local" before 26.9.30. -func validateWireguardRemoteDNS(settings *serial.TypedMessage) error { +// validateHandlerSettings refuses what conf.Build() lets through but the core parses only +// in the handler constructor, where one failure aborts the whole core start. +func validateHandlerSettings(settings *serial.TypedMessage) error { if settings == nil { return nil } @@ -205,10 +214,42 @@ func validateWireguardRemoteDNS(settings *serial.TypedMessage) error { if err != nil { return nil } - device, ok := instance.(*wireguard.DeviceConfig) - if !ok || !device.IsClient { + switch config := instance.(type) { + case *wireguard.DeviceConfig: + return validateWireguardClient(config) + case *vlessOutbound.Config: + return validateVlessEncryption(config) + case *shadowsocks_2022.ClientConfig: + return validateShadowsocks2022Key(config) + } + return nil +} + +// validateWireguardClient mirrors proxy/wireguard.NewClient; remoteDNS goes to +// netip.MustParseAddr there, so a non-IP entry ("local" before 26.9.30) is a crash. +func validateWireguardClient(device *wireguard.DeviceConfig) error { + if !device.IsClient { return nil } + if len(device.Peers) == 0 { + return common.NewError("wireguard outbound has no peers") + } + for _, peer := range device.Peers { + if peer.PublicKey == "" { + return common.NewError("wireguard peer without publicKey") + } + if peer.Endpoint == "" { + return common.NewError("wireguard peer without endpoint") + } + } + for _, local := range device.Endpoint { + if _, err := netip.ParseAddr(local); err == nil { + continue + } + if _, err := netip.ParsePrefix(local); err != nil { + return common.NewErrorf("wireguard address entry %q is neither an IP nor a prefix", local) + } + } for _, server := range device.DNS { if _, err := netip.ParseAddr(server); err != nil { return common.NewErrorf("wireguard remoteDNS entry %q is not an IP address", server) @@ -217,6 +258,48 @@ func validateWireguardRemoteDNS(settings *serial.TypedMessage) error { return nil } +// validateVlessEncryption mirrors proxy/vless/outbound.New, which parses the padding spec +// and the X25519 / ML-KEM-768 keys of "encryption" through ClientInstance.Init. +func validateVlessEncryption(config *vlessOutbound.Config) error { + if config.Vnext == nil || config.Vnext.User == nil { + return nil + } + user, err := config.Vnext.User.ToMemoryUser() + if err != nil { + return err + } + account, ok := user.Account.(*vless.MemoryAccount) + if !ok || account.Encryption == "" || account.Encryption == "none" { + return nil + } + var keys [][]byte + for part := range strings.SplitSeq(account.Encryption, ".") { + key, _ := base64.RawURLEncoding.DecodeString(part) + keys = append(keys, key) + } + if err := (&vlessEncryption.ClientInstance{}).Init(keys, account.XorMode, account.Seconds, account.Padding); err != nil { + return common.NewErrorf("vless encryption is unusable: %v", err) + } + return nil +} + +// validateShadowsocks2022Key mirrors shadowsocks_2022.NewClient: every ":"-separated key +// is base64 of the cipher's key length, and chacha20-poly1305 takes a single key. +func validateShadowsocks2022Key(config *shadowsocks_2022.ClientConfig) error { + method, err := shadowsocks_2022.GetCipherMethod(config.Method) + if err != nil { + return err + } + keys, err := shadowsocks_2022.ParsePSKList(config.Key, method.KeySaltLength) + if err != nil { + return common.NewErrorf("shadowsocks 2022 key is unusable for %s: %v", config.Method, err) + } + if method.IsChaCha && len(keys) > 1 { + return common.NewError("shadowsocks 2022 chacha20-poly1305 does not support multiple keys") + } + return nil +} + // AddOutbound adds a new outbound configuration to the Xray core via gRPC. func (x *XrayAPI) AddOutbound(outbound []byte) error { if x.HandlerServiceClient == nil { diff --git a/internal/xray/outbound_validation_test.go b/internal/xray/outbound_validation_test.go index 11a506f3b..34d2717e7 100644 --- a/internal/xray/outbound_validation_test.go +++ b/internal/xray/outbound_validation_test.go @@ -1,10 +1,13 @@ package xray import ( + "encoding/json" "os" "path/filepath" "strings" "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/util/link" ) // TestValidateOutboundConfig_RejectsUnencryptedPublicVless covers xray-core @@ -86,3 +89,64 @@ func TestValidateOutboundConfig_FindsANoiseScriptBesideTheXrayBinary(t *testing. t.Fatalf("ValidateOutboundConfig = %v, want the script beside the xray binary found", err) } } + +// The core's loader panics on some third-party values instead of returning an error; +// a subscription refresh must count that link as rejected, not unwind the whole job. +func TestValidateOutboundConfig_ReturnsTheLoaderPanicAsAnError(t *testing.T) { + for name, outbound := range map[string]string{ + "vless encryption": `{"protocol": "vless", + "settings": {"address": "1.2.3.4", "port": 443, "id": "b831381d-6324-4d53-ad4f-8cda48b30811", "encryption": "mlkem768x25519plus.native.0rtt.AAAA"}, + "streamSettings": {"network": "tcp", "security": "tls", "tlsSettings": {"serverName": "example.com"}}}`, + "reality spiderX": `{"protocol": "vless", + "settings": {"address": "1.2.3.4", "port": 443, "id": "b831381d-6324-4d53-ad4f-8cda48b30811", "encryption": "none"}, + "streamSettings": {"network": "tcp", "security": "reality", "realitySettings": { + "serverName": "example.com", "fingerprint": "chrome", "spiderX": "/%zz", "publicKey": "xTIBA5rboUvnH4htodjb6e697QjLERt1NAB4mZqp8Dg"}}}`, + } { + if err := ValidateOutboundConfig([]byte(outbound)); err == nil || !strings.Contains(err.Error(), "panicked") { + t.Fatalf("%s: ValidateOutboundConfig = %v, want the loader's panic as an error", name, err) + } + } +} + +// conf.Build() lets these through, but the handler constructor parses them at core +// start, and one failure there takes every inbound down with it. +func TestValidateOutboundConfig_RejectsLinksTheHandlerConstructorRefuses(t *testing.T) { + const wg = "wireguard://JOSLNr3Kk8BViwrrTJ2M19gRBbGttnRfApY3nQ3fdT8%3D@203.0.113.10:51820?" + const wgPeer = "publickey=biSnuvUOX2pmtzf249FK8UGSq2PQslGSdoj4UqH4yew%3D" + const vless = "vless://b831381d-6324-4d53-ad4f-8cda48b30811@203.0.113.10:443?security=tls&sni=a&encryption=mlkem768x25519plus.native.1rtt." + x25519 := strings.Repeat("A", 43) + // ss userinfo: "2022-blake3-aes-128-gcm:shortkey", "hello world" (method guessed), a 16-byte key + const ssShort = "ss://MjAyMi1ibGFrZTMtYWVzLTEyOC1nY206c2hvcnRrZXk@203.0.113.10:8388" + const ssGuessed = "ss://aGVsbG8gd29ybGQ@203.0.113.10:8388" + const ssValid = "ss://MjAyMi1ibGFrZTMtYWVzLTEyOC1nY206QndjSEJ3Y0hCd2NIQndjSEJ3Y0hCdz09@203.0.113.10:8388" + for _, tc := range []struct{ name, link, wantErr string }{ + {"wireguard address", wg + wgPeer + "&address=garbage", `address entry "garbage"`}, + {"wireguard no public key", wg + "address=10.0.0.2/32", "peer without publicKey"}, + {"wireguard valid", wg + wgPeer + "&address=10.0.0.2/32", ""}, + {"vless padding", vless + "abc." + x25519, "vless encryption"}, + {"vless mlkem key", vless + strings.Repeat("_", 1579), "vless encryption"}, + {"vless valid", vless + x25519, ""}, + {"ss2022 short key", ssShort, "shadowsocks 2022 key"}, + {"ss guessed 2022 method", ssGuessed, "shadowsocks 2022 key"}, + {"ss2022 valid", ssValid, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + parsed, err := link.ParseLink(tc.link) + if err != nil { + t.Fatalf("ParseLink: %v", err) + } + parsed.Outbound["tag"] = "t" + raw, _ := json.Marshal(parsed.Outbound) + err = ValidateOutboundConfig(raw) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("ValidateOutboundConfig(%s) = %v, want nil", raw, err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("ValidateOutboundConfig(%s) = %v, want an error containing %q", raw, err, tc.wantErr) + } + }) + } +}