From cf36159aa8477587d381e8e58ce608c19c73a9c6 Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 29 Jul 2026 18:57:26 +0200 Subject: [PATCH] Fix PROXY header handling to prevent panic on missing source address --- server.go | 8 ++++++++ server_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/server.go b/server.go index b3860a5..b6b4dce 100644 --- a/server.go +++ b/server.go @@ -476,6 +476,14 @@ func (s *server) handleClient(client *client) { client.RemoteIP = "" s.log().Infof("Proxying from %s", proto) case "TCP4", "TCP6": + // A well-formed v1 header carries 5 params after the protocol + // token; we only read the source address, so require at least + // that one rather than trusting the peer to send them all. + if len(params) < 2 { + s.log().Errorf("PROXY header specified protocol %s but omitted the source address", proto) + client.kill() + return + } ipStr := string(params[1]) if ip, err := netip.ParseAddr(ipStr); err != nil { s.log().WithError(err).Errorf("Invalid IP address in PROXY header from %s: %s", client.RemoteIP, string(params[1])) diff --git a/server_test.go b/server_test.go index a701026..366ca11 100644 --- a/server_test.go +++ b/server_test.go @@ -13,6 +13,7 @@ import ( "fmt" "net" + "time" "github.com/phires/go-guerrilla/backends" "github.com/phires/go-guerrilla/log" @@ -965,6 +966,54 @@ func TestProxy(t *testing.T) { wg.Wait() // wait for handleClient to exit } +// A PROXY header that announces TCP4/TCP6 but omits the source address must not +// take the process down. Before the fix, params[1] was indexed after the +// "PROXY" prefix had been stripped, so a header carrying only the protocol +// token panicked the connection goroutine and, with no recover() in the +// connection path, the whole daemon. +func TestProxyMissingAddress(t *testing.T) { + for _, header := range []string{"PROXY TCP4", "PROXY TCP6"} { + t.Run(header, func(t *testing.T) { + defer cleanTestArtifacts(t) + sc := getMockServerConfig() + sc.ProxyOn = true + mainlog, logOpenError := log.GetLogger(sc.LogFile, "debug") + if logOpenError != nil { + mainlog.WithError(logOpenError).Errorf("Failed creating a logger for mock conn [%s]", sc.ListenInterface) + } + conn, server := getMockServerConn(sc, t) + client := NewClient(conn.Server, 1, mainlog, mail.NewPool(5)) + var wg sync.WaitGroup + wg.Add(1) + go func() { + server.handleClient(client) + wg.Done() + }() + + w := textproto.NewWriter(bufio.NewWriter(conn.Client)) + if err := w.PrintfLine("%s", header); err != nil { + t.Error(err) + } + + // The server must reject the header and hang up rather than panic. + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("handleClient did not return after a malformed PROXY header") + } + + if client.isAlive() { + t.Error("client should have been killed after a malformed PROXY header") + } + }) + } +} + // The backend gateway should time out after 1 second because it sleeps for 2 sec. // The transaction should wait until finished, and then test to see if we can do // a second transaction