From 3f7e141d2e51e59c7f747fd3b35d167e42909170 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 15 Jul 2026 04:47:38 +0200 Subject: [PATCH] fix(server): guard xray key-generator output parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetNewX25519Cert, GetNewmldsa65 and GetNewmlkem768 parsed xray's stdout by reading lines[0], lines[1] and each line's second colon-separated field without any length check — unlike GetNewEchCert, which already guards its line count. If the xray binary printed fewer than two lines or reformatted its labels (a version change, or a silent failure that emitted nothing), the fixed slice index panicked and the handler 500'd. Extract the shared parsing into parseXrayKeyPairOutput, which length guards the line count and each label split and returns an error instead of panicking, then route all three generators through it. --- internal/web/service/key_gen_parse_test.go | 37 ++++++++++++++++ internal/web/service/server.go | 51 +++++++++++++--------- 2 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 internal/web/service/key_gen_parse_test.go diff --git a/internal/web/service/key_gen_parse_test.go b/internal/web/service/key_gen_parse_test.go new file mode 100644 index 000000000..68aeb3f69 --- /dev/null +++ b/internal/web/service/key_gen_parse_test.go @@ -0,0 +1,37 @@ +package service + +import "testing" + +// The x25519 / mldsa65 / mlkem768 key generators parse xray's stdout by fixed +// slice index. A short or reformatted output (a future xray release, or a +// binary that failed silently) must yield an error, never an out-of-range +// panic that 500s the handler. +func TestParseXrayKeyPairOutput(t *testing.T) { + a, b, err := parseXrayKeyPairOutput("Private key: abc123\nPublic key: def456\n") + if err != nil { + t.Fatalf("well-formed output errored: %v", err) + } + if a != "abc123" || b != "def456" { + t.Fatalf("got (%q, %q), want (abc123, def456)", a, b) + } + + malformed := []string{ + "", + "only one line: value", + "Private key: abc\n", + "no colon here\nno colon two", + "Private key\nPublic key", + } + for _, out := range malformed { + func() { + defer func() { + if r := recover(); r != nil { + t.Errorf("parseXrayKeyPairOutput panicked on %q: %v", out, r) + } + }() + if _, _, err := parseXrayKeyPairOutput(out); err == nil { + t.Errorf("expected error for malformed output %q, got nil", out) + } + }() + } +} diff --git a/internal/web/service/server.go b/internal/web/service/server.go index dd190d4b2..7afa144ca 100644 --- a/internal/web/service/server.go +++ b/internal/web/service/server.go @@ -2023,6 +2023,24 @@ func (s *ServerService) UpdateGeofile(fileName string) error { return nil } +// parseXrayKeyPairOutput reads the two-line "Label: value" output that xray's +// key-generation subcommands (x25519, mldsa65, mlkem768) print and returns the +// two values. Short or label-less output yields an error instead of panicking +// on an out-of-range slice index, so a future xray version that changes the +// format degrades to a 500 with a message rather than a crash. +func parseXrayKeyPairOutput(output string) (string, string, error) { + lines := strings.Split(output, "\n") + if len(lines) < 2 { + return "", "", common.NewError("unexpected key generator output") + } + first := strings.Split(lines[0], ":") + second := strings.Split(lines[1], ":") + if len(first) < 2 || len(second) < 2 { + return "", "", common.NewError("unexpected key generator output") + } + return strings.TrimSpace(first[1]), strings.TrimSpace(second[1]), nil +} + func (s *ServerService) GetNewX25519Cert() (any, error) { // Run the command cmd := exec.CommandContext(context.Background(), xray.GetBinaryPath(), "x25519") @@ -2033,13 +2051,10 @@ func (s *ServerService) GetNewX25519Cert() (any, error) { return nil, err } - lines := strings.Split(out.String(), "\n") - - privateKeyLine := strings.Split(lines[0], ":") - publicKeyLine := strings.Split(lines[1], ":") - - privateKey := strings.TrimSpace(privateKeyLine[1]) - publicKey := strings.TrimSpace(publicKeyLine[1]) + privateKey, publicKey, err := parseXrayKeyPairOutput(out.String()) + if err != nil { + return nil, err + } keyPair := map[string]any{ "privateKey": privateKey, @@ -2059,13 +2074,10 @@ func (s *ServerService) GetNewmldsa65() (any, error) { return nil, err } - lines := strings.Split(out.String(), "\n") - - SeedLine := strings.Split(lines[0], ":") - VerifyLine := strings.Split(lines[1], ":") - - seed := strings.TrimSpace(SeedLine[1]) - verify := strings.TrimSpace(VerifyLine[1]) + seed, verify, err := parseXrayKeyPairOutput(out.String()) + if err != nil { + return nil, err + } keyPair := map[string]any{ "seed": seed, @@ -2381,13 +2393,10 @@ func (s *ServerService) GetNewmlkem768() (any, error) { return nil, err } - lines := strings.Split(out.String(), "\n") - - SeedLine := strings.Split(lines[0], ":") - ClientLine := strings.Split(lines[1], ":") - - seed := strings.TrimSpace(SeedLine[1]) - client := strings.TrimSpace(ClientLine[1]) + seed, client, err := parseXrayKeyPairOutput(out.String()) + if err != nil { + return nil, err + } keyPair := map[string]any{ "seed": seed,