mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-06-08 14:14:19 +00:00
fix: Clash YAML injection, path validation, and default template
- Quote all YAML string values with %q to prevent injection - Remove unused host parameter from GetClash - Add backend path normalization for SubClashPath - Log StreamSettings JSON unmarshal errors - Expand template panel by default and provide default template
This commit is contained in:
parent
11cdb07e89
commit
288635adfc
7 changed files with 80 additions and 34 deletions
|
|
@ -1 +1 @@
|
||||||
v1.5.2-beta
|
v1.5.3-beta
|
||||||
18
docs/Tasktracking/2026-04-24-fix-clash-yaml-code-quality.md
Normal file
18
docs/Tasktracking/2026-04-24-fix-clash-yaml-code-quality.md
Normal file
|
|
@ -0,0 +1,18 @@
|
||||||
|
# Fix Clash YAML Subscription Code Quality Issues
|
||||||
|
|
||||||
|
**Date:** 2026-04-24
|
||||||
|
**Type:** bugfix / code-quality
|
||||||
|
**Version:** v1.5.3-beta
|
||||||
|
|
||||||
|
## Issues Fixed
|
||||||
|
|
||||||
|
- **YAML injection (C2):** All string values in Clash YAML generation changed from `%s` to `%q` for proper quoting
|
||||||
|
- **Unused parameter (I1):** Removed unused `host` parameter from `GetClash` method
|
||||||
|
- **Path validation (I3):** Added backend normalization for `SubClashPath` (auto-add leading/trailing `/`)
|
||||||
|
- **Silent JSON error (I4):** Added warning log when `StreamSettings` JSON unmarshal fails
|
||||||
|
- **Template visibility:** Clash YAML template panel now expanded by default
|
||||||
|
- **Default template:** Added sensible default Clash YAML template in settings defaults
|
||||||
|
|
||||||
|
## Note
|
||||||
|
|
||||||
|
C1 from review (nil `inboundService` panic) was a false alarm — `InboundService` is a value type, not a pointer.
|
||||||
|
|
@ -26,8 +26,8 @@ func NewSubClashService(template string, subService *SubService) *SubClashServic
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetClash generates a Clash YAML configuration for the given subscription ID and host.
|
// GetClash generates a Clash YAML configuration for the given subscription ID.
|
||||||
func (s *SubClashService) GetClash(subId string, host string) (string, string, error) {
|
func (s *SubClashService) GetClash(subId string) (string, string, error) {
|
||||||
if s.template == "" {
|
if s.template == "" {
|
||||||
return "", "", fmt.Errorf("clash template is empty")
|
return "", "", fmt.Errorf("clash template is empty")
|
||||||
}
|
}
|
||||||
|
|
@ -112,7 +112,9 @@ func (s *SubClashService) GetClash(subId string, host string) (string, string, e
|
||||||
func (s *SubClashService) getProxy(inbound *model.Inbound, client model.Client) []string {
|
func (s *SubClashService) getProxy(inbound *model.Inbound, client model.Client) []string {
|
||||||
var proxies []string
|
var proxies []string
|
||||||
var stream map[string]any
|
var stream map[string]any
|
||||||
json.Unmarshal([]byte(inbound.StreamSettings), &stream)
|
if err := json.Unmarshal([]byte(inbound.StreamSettings), &stream); err != nil {
|
||||||
|
logger.Warning("SubClashService - failed to parse StreamSettings for inbound", inbound.Tag, ":", err)
|
||||||
|
}
|
||||||
|
|
||||||
// Resolve address
|
// Resolve address
|
||||||
var address string
|
var address string
|
||||||
|
|
@ -184,34 +186,34 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
switch inbound.Protocol {
|
switch inbound.Protocol {
|
||||||
case model.VMESS:
|
case model.VMESS:
|
||||||
parts = append(parts, "type: vmess")
|
parts = append(parts, "type: vmess")
|
||||||
parts = append(parts, fmt.Sprintf("server: %s", address))
|
parts = append(parts, fmt.Sprintf("server: %q", address))
|
||||||
parts = append(parts, fmt.Sprintf("port: %d", port))
|
parts = append(parts, fmt.Sprintf("port: %d", port))
|
||||||
parts = append(parts, fmt.Sprintf("uuid: %s", client.ID))
|
parts = append(parts, fmt.Sprintf("uuid: %q", client.ID))
|
||||||
parts = append(parts, "alterId: 0")
|
parts = append(parts, "alterId: 0")
|
||||||
parts = append(parts, "cipher: auto")
|
parts = append(parts, "cipher: auto")
|
||||||
|
|
||||||
case model.VLESS:
|
case model.VLESS:
|
||||||
parts = append(parts, "type: vless")
|
parts = append(parts, "type: vless")
|
||||||
parts = append(parts, fmt.Sprintf("server: %s", address))
|
parts = append(parts, fmt.Sprintf("server: %q", address))
|
||||||
parts = append(parts, fmt.Sprintf("port: %d", port))
|
parts = append(parts, fmt.Sprintf("port: %d", port))
|
||||||
parts = append(parts, fmt.Sprintf("uuid: %s", client.ID))
|
parts = append(parts, fmt.Sprintf("uuid: %q", client.ID))
|
||||||
if client.Flow != "" {
|
if client.Flow != "" {
|
||||||
parts = append(parts, fmt.Sprintf("flow: %s", client.Flow))
|
parts = append(parts, fmt.Sprintf("flow: %q", client.Flow))
|
||||||
}
|
}
|
||||||
|
|
||||||
case model.Trojan:
|
case model.Trojan:
|
||||||
parts = append(parts, "type: trojan")
|
parts = append(parts, "type: trojan")
|
||||||
parts = append(parts, fmt.Sprintf("server: %s", address))
|
parts = append(parts, fmt.Sprintf("server: %q", address))
|
||||||
parts = append(parts, fmt.Sprintf("port: %d", port))
|
parts = append(parts, fmt.Sprintf("port: %d", port))
|
||||||
parts = append(parts, fmt.Sprintf("password: %s", client.Password))
|
parts = append(parts, fmt.Sprintf("password: %q", client.Password))
|
||||||
|
|
||||||
case model.Shadowsocks:
|
case model.Shadowsocks:
|
||||||
parts = append(parts, "type: ss")
|
parts = append(parts, "type: ss")
|
||||||
parts = append(parts, fmt.Sprintf("server: %s", address))
|
parts = append(parts, fmt.Sprintf("server: %q", address))
|
||||||
parts = append(parts, fmt.Sprintf("port: %d", port))
|
parts = append(parts, fmt.Sprintf("port: %d", port))
|
||||||
cipher, password := s.parseShadowsocksSettings(client)
|
cipher, password := s.parseShadowsocksSettings(client)
|
||||||
parts = append(parts, fmt.Sprintf("cipher: %s", cipher))
|
parts = append(parts, fmt.Sprintf("cipher: %q", cipher))
|
||||||
parts = append(parts, fmt.Sprintf("password: %s", password))
|
parts = append(parts, fmt.Sprintf("password: %q", password))
|
||||||
parts = append(parts, "udp: true")
|
parts = append(parts, "udp: true")
|
||||||
return strings.Join(parts, "\n ")
|
return strings.Join(parts, "\n ")
|
||||||
}
|
}
|
||||||
|
|
@ -222,9 +224,9 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
if security == "reality" {
|
if security == "reality" {
|
||||||
realitySetting, _ := stream["realitySettings"].(map[string]any)
|
realitySetting, _ := stream["realitySettings"].(map[string]any)
|
||||||
if publicKey, ok := realitySetting["publicKey"].(string); ok && publicKey != "" {
|
if publicKey, ok := realitySetting["publicKey"].(string); ok && publicKey != "" {
|
||||||
realityOpts := fmt.Sprintf("reality-opts:\n public-key: %s", publicKey)
|
realityOpts := fmt.Sprintf("reality-opts:\n public-key: %q", publicKey)
|
||||||
if shortId, ok := realitySetting["shortId"].(string); ok && shortId != "" {
|
if shortId, ok := realitySetting["shortId"].(string); ok && shortId != "" {
|
||||||
realityOpts += fmt.Sprintf("\n short-id: %s", shortId)
|
realityOpts += fmt.Sprintf("\n short-id: %q", shortId)
|
||||||
}
|
}
|
||||||
parts = append(parts, realityOpts)
|
parts = append(parts, realityOpts)
|
||||||
}
|
}
|
||||||
|
|
@ -232,13 +234,13 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
serverNames, _ := realitySetting["serverNames"].([]any)
|
serverNames, _ := realitySetting["serverNames"].([]any)
|
||||||
if len(serverNames) > 0 {
|
if len(serverNames) > 0 {
|
||||||
sni := fmt.Sprintf("%v", serverNames[0])
|
sni := fmt.Sprintf("%v", serverNames[0])
|
||||||
parts = append(parts, fmt.Sprintf("sni: %s", sni))
|
parts = append(parts, fmt.Sprintf("sni: %q", sni))
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// TLS settings
|
// TLS settings
|
||||||
tlsSetting, _ := stream["tlsSettings"].(map[string]any)
|
tlsSetting, _ := stream["tlsSettings"].(map[string]any)
|
||||||
if serverName, ok := tlsSetting["serverName"].(string); ok && serverName != "" {
|
if serverName, ok := tlsSetting["serverName"].(string); ok && serverName != "" {
|
||||||
parts = append(parts, fmt.Sprintf("sni: %s", serverName))
|
parts = append(parts, fmt.Sprintf("sni: %q", serverName))
|
||||||
}
|
}
|
||||||
if alpn, ok := tlsSetting["alpn"].([]any); ok && len(alpn) > 0 {
|
if alpn, ok := tlsSetting["alpn"].([]any); ok && len(alpn) > 0 {
|
||||||
alpnStrs := make([]string, len(alpn))
|
alpnStrs := make([]string, len(alpn))
|
||||||
|
|
@ -250,7 +252,7 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
}
|
}
|
||||||
// Fingerprint
|
// Fingerprint
|
||||||
if fp, ok := stream["fingerprint"].(string); ok && fp != "" {
|
if fp, ok := stream["fingerprint"].(string); ok && fp != "" {
|
||||||
parts = append(parts, fmt.Sprintf("client-fingerprint: %s", fp))
|
parts = append(parts, fmt.Sprintf("client-fingerprint: %q", fp))
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
parts = append(parts, "tls: false")
|
parts = append(parts, "tls: false")
|
||||||
|
|
@ -263,13 +265,13 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
case "ws":
|
case "ws":
|
||||||
ws, _ := stream["wsSettings"].(map[string]any)
|
ws, _ := stream["wsSettings"].(map[string]any)
|
||||||
if path, ok := ws["path"].(string); ok && path != "" {
|
if path, ok := ws["path"].(string); ok && path != "" {
|
||||||
wsOpts := fmt.Sprintf("ws-opts:\n path: %s", path)
|
wsOpts := fmt.Sprintf("ws-opts:\n path: %q", path)
|
||||||
if host, ok := ws["host"].(string); ok && host != "" {
|
if host, ok := ws["host"].(string); ok && host != "" {
|
||||||
wsOpts += fmt.Sprintf("\n headers:\n Host: %s", host)
|
wsOpts += fmt.Sprintf("\n headers:\n Host: %q", host)
|
||||||
} else {
|
} else {
|
||||||
headers, _ := ws["headers"].(map[string]any)
|
headers, _ := ws["headers"].(map[string]any)
|
||||||
if h, ok := headers["Host"].(string); ok && h != "" {
|
if h, ok := headers["Host"].(string); ok && h != "" {
|
||||||
wsOpts += fmt.Sprintf("\n headers:\n Host: %s", h)
|
wsOpts += fmt.Sprintf("\n headers:\n Host: %q", h)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
parts = append(parts, wsOpts)
|
parts = append(parts, wsOpts)
|
||||||
|
|
@ -278,17 +280,17 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
case "grpc":
|
case "grpc":
|
||||||
grpc, _ := stream["grpcSettings"].(map[string]any)
|
grpc, _ := stream["grpcSettings"].(map[string]any)
|
||||||
if serviceName, ok := grpc["serviceName"].(string); ok && serviceName != "" {
|
if serviceName, ok := grpc["serviceName"].(string); ok && serviceName != "" {
|
||||||
parts = append(parts, fmt.Sprintf("grpc-opts:\n grpc-service-name: %s", serviceName))
|
parts = append(parts, fmt.Sprintf("grpc-opts:\n grpc-service-name: %q", serviceName))
|
||||||
}
|
}
|
||||||
|
|
||||||
case "h2":
|
case "h2":
|
||||||
h2, _ := stream["h2Settings"].(map[string]any)
|
h2, _ := stream["h2Settings"].(map[string]any)
|
||||||
if path, ok := h2["path"].(string); ok && path != "" {
|
if path, ok := h2["path"].(string); ok && path != "" {
|
||||||
h2Opts := fmt.Sprintf("h2-opts:\n path: %s", path)
|
h2Opts := fmt.Sprintf("h2-opts:\n path: %q", path)
|
||||||
if host, ok := h2["host"].([]any); ok && len(host) > 0 {
|
if host, ok := h2["host"].([]any); ok && len(host) > 0 {
|
||||||
hostStrs := make([]string, len(host))
|
hostStrs := make([]string, len(host))
|
||||||
for i, h := range host {
|
for i, h := range host {
|
||||||
hostStrs[i] = fmt.Sprintf("%v", h)
|
hostStrs[i] = fmt.Sprintf("%q", fmt.Sprintf("%v", h))
|
||||||
}
|
}
|
||||||
h2Opts += fmt.Sprintf("\n host: [%s]", strings.Join(hostStrs, ", "))
|
h2Opts += fmt.Sprintf("\n host: [%s]", strings.Join(hostStrs, ", "))
|
||||||
}
|
}
|
||||||
|
|
@ -302,13 +304,13 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
request, _ := header["request"].(map[string]any)
|
request, _ := header["request"].(map[string]any)
|
||||||
httpOpts := "http-opts:"
|
httpOpts := "http-opts:"
|
||||||
if path, ok := request["path"].([]any); ok && len(path) > 0 {
|
if path, ok := request["path"].([]any); ok && len(path) > 0 {
|
||||||
httpOpts += fmt.Sprintf("\n path:\n - %v", path[0])
|
httpOpts += fmt.Sprintf("\n path:\n - %q", fmt.Sprintf("%v", path[0]))
|
||||||
}
|
}
|
||||||
if headers, ok := request["headers"].(map[string]any); ok && len(headers) > 0 {
|
if headers, ok := request["headers"].(map[string]any); ok && len(headers) > 0 {
|
||||||
httpOpts += "\n headers:"
|
httpOpts += "\n headers:"
|
||||||
for k, v := range headers {
|
for k, v := range headers {
|
||||||
if vals, ok := v.([]any); ok && len(vals) > 0 {
|
if vals, ok := v.([]any); ok && len(vals) > 0 {
|
||||||
httpOpts += fmt.Sprintf("\n %s:\n - %v", k, vals[0])
|
httpOpts += fmt.Sprintf("\n %s:\n - %q", k, fmt.Sprintf("%v", vals[0]))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -318,13 +320,13 @@ func (s *SubClashService) buildProxyEntry(inbound *model.Inbound, client model.C
|
||||||
case "httpupgrade":
|
case "httpupgrade":
|
||||||
hu, _ := stream["httpupgradeSettings"].(map[string]any)
|
hu, _ := stream["httpupgradeSettings"].(map[string]any)
|
||||||
if path, ok := hu["path"].(string); ok && path != "" {
|
if path, ok := hu["path"].(string); ok && path != "" {
|
||||||
huOpts := fmt.Sprintf("httpupgrade-opts:\n path: %s", path)
|
huOpts := fmt.Sprintf("httpupgrade-opts:\n path: %q", path)
|
||||||
if host, ok := hu["host"].(string); ok && host != "" {
|
if host, ok := hu["host"].(string); ok && host != "" {
|
||||||
huOpts += fmt.Sprintf("\n host: %s", host)
|
huOpts += fmt.Sprintf("\n host: %q", host)
|
||||||
} else {
|
} else {
|
||||||
headers, _ := hu["headers"].(map[string]any)
|
headers, _ := hu["headers"].(map[string]any)
|
||||||
if h, ok := headers["Host"].(string); ok && h != "" {
|
if h, ok := headers["Host"].(string); ok && h != "" {
|
||||||
huOpts += fmt.Sprintf("\n host: %s", h)
|
huOpts += fmt.Sprintf("\n host: %q", h)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
parts = append(parts, huOpts)
|
parts = append(parts, huOpts)
|
||||||
|
|
|
||||||
|
|
@ -199,8 +199,8 @@ func (a *SUBController) subJsons(c *gin.Context) {
|
||||||
// clashSubs handles HTTP requests for Clash YAML subscription configurations.
|
// clashSubs handles HTTP requests for Clash YAML subscription configurations.
|
||||||
func (a *SUBController) clashSubs(c *gin.Context) {
|
func (a *SUBController) clashSubs(c *gin.Context) {
|
||||||
subId := c.Param("subid")
|
subId := c.Param("subid")
|
||||||
scheme, host, hostWithPort, _ := a.subService.ResolveRequest(c)
|
scheme, _, hostWithPort, _ := a.subService.ResolveRequest(c)
|
||||||
clashYaml, header, err := a.subClashService.GetClash(subId, host)
|
clashYaml, header, err := a.subClashService.GetClash(subId)
|
||||||
if err != nil || len(clashYaml) == 0 {
|
if err != nil || len(clashYaml) == 0 {
|
||||||
c.String(400, "Error!")
|
c.String(400, "Error!")
|
||||||
} else {
|
} else {
|
||||||
|
|
|
||||||
|
|
@ -185,6 +185,13 @@ func (s *AllSetting) CheckValid() error {
|
||||||
s.SubJsonPath += "/"
|
s.SubJsonPath += "/"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !strings.HasPrefix(s.SubClashPath, "/") {
|
||||||
|
s.SubClashPath = "/" + s.SubClashPath
|
||||||
|
}
|
||||||
|
if !strings.HasSuffix(s.SubClashPath, "/") {
|
||||||
|
s.SubClashPath += "/"
|
||||||
|
}
|
||||||
|
|
||||||
_, err := time.LoadLocation(s.TimeLocation)
|
_, err := time.LoadLocation(s.TimeLocation)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return common.NewError("time location not exist:", s.TimeLocation)
|
return common.NewError("time location not exist:", s.TimeLocation)
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
{{define "settings/panel/subscription/clash"}}
|
{{define "settings/panel/subscription/clash"}}
|
||||||
<a-collapse default-active-key="1">
|
<a-collapse default-active-key="['1','2']">
|
||||||
<a-collapse-panel key="1" header='{{ i18n "pages.xray.generalConfigs"}}'>
|
<a-collapse-panel key="1" header='{{ i18n "pages.xray.generalConfigs"}}'>
|
||||||
<a-setting-list-item paddings="small">
|
<a-setting-list-item paddings="small">
|
||||||
<template #title>{{ i18n "pages.settings.subPath"}}</template>
|
<template #title>{{ i18n "pages.settings.subPath"}}</template>
|
||||||
|
|
|
||||||
|
|
@ -81,7 +81,26 @@ var defaultValueMap = map[string]string{
|
||||||
"subClashEnable": "false",
|
"subClashEnable": "false",
|
||||||
"subClashPath": "/clash/",
|
"subClashPath": "/clash/",
|
||||||
"subClashURI": "",
|
"subClashURI": "",
|
||||||
"subClashTemplate": "",
|
"subClashTemplate": `port: 7890
|
||||||
|
socks-port: 7891
|
||||||
|
allow-lan: false
|
||||||
|
mode: rule
|
||||||
|
log-level: info
|
||||||
|
proxies: []
|
||||||
|
proxy-groups:
|
||||||
|
- name: Proxy
|
||||||
|
type: select
|
||||||
|
proxies:
|
||||||
|
- DIRECT
|
||||||
|
dns:
|
||||||
|
enable: true
|
||||||
|
enhanced-mode: fake-ip
|
||||||
|
nameserver:
|
||||||
|
- 8.8.8.8
|
||||||
|
- 1.1.1.1
|
||||||
|
rules:
|
||||||
|
- GEOIP,LAN,DIRECT
|
||||||
|
- MATCH,Proxy`,
|
||||||
"datepicker": "gregorian",
|
"datepicker": "gregorian",
|
||||||
"warp": "",
|
"warp": "",
|
||||||
"externalTrafficInformEnable": "false",
|
"externalTrafficInformEnable": "false",
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue