From 68a981c0c331dd93ccde20c4abbffed5608454e9 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 15 Jul 2026 04:44:53 +0200 Subject: [PATCH] fix(server): guard access-log parser against malformed lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetXrayLogs split each Xray access-log line on whitespace and then read fixed offsets — parts[1] for the timestamp and parts[i+1] after the "from", "accepted" and "email:" markers — without checking the line had that many fields. A truncated or malformed line (the logged destination is attacker-influenced) indexed past the slice and panicked; the panel handler returned a 500 via Gin's recovery. Extract the per-line field parsing into parseAccessLogFields and length guard every positional lookup so a short line yields a partial entry instead of panicking. --- internal/web/service/access_log_parse_test.go | 49 +++++++++++++++ internal/web/service/server.go | 60 +++++++++++-------- 2 files changed, 84 insertions(+), 25 deletions(-) create mode 100644 internal/web/service/access_log_parse_test.go diff --git a/internal/web/service/access_log_parse_test.go b/internal/web/service/access_log_parse_test.go new file mode 100644 index 000000000..26dbecf1b --- /dev/null +++ b/internal/web/service/access_log_parse_test.go @@ -0,0 +1,49 @@ +package service + +import "testing" + +// Xray access-log lines carry attacker-influenced content (a client's requested +// destination is logged verbatim) and can be truncated. parseAccessLogFields +// must never panic on a short or malformed line, and must still parse a +// well-formed line correctly. +func TestParseAccessLogFields(t *testing.T) { + malformed := []string{ + "", + "singletoken", + "2024/01/02", + "2024/01/02 15:04:05.000000 from", + "2024/01/02 15:04:05.000000 accepted", + "2024/01/02 15:04:05.000000 email:", + } + for _, line := range malformed { + func() { + defer func() { + if r := recover(); r != nil { + t.Errorf("parseAccessLogFields panicked on %q: %v", line, r) + } + }() + _ = parseAccessLogFields(line) + }() + } + + line := "2024/01/02 15:04:05.123456 from 1.2.3.4:555 accepted tcp:example.com:443 [inbound-tag >> outbound-tag] email: alice@example.com" + entry := parseAccessLogFields(line) + if entry.FromAddress != "1.2.3.4:555" { + t.Errorf("FromAddress = %q, want %q", entry.FromAddress, "1.2.3.4:555") + } + if entry.ToAddress != "tcp:example.com:443" { + t.Errorf("ToAddress = %q, want %q", entry.ToAddress, "tcp:example.com:443") + } + if entry.Inbound != "inbound-tag" { + t.Errorf("Inbound = %q, want %q", entry.Inbound, "inbound-tag") + } + if entry.Outbound != "outbound-tag" { + t.Errorf("Outbound = %q, want %q", entry.Outbound, "outbound-tag") + } + if entry.Email != "alice@example.com" { + t.Errorf("Email = %q, want %q", entry.Email, "alice@example.com") + } + if entry.DateTime.IsZero() { + t.Error("DateTime was not parsed from a well-formed line") + } +} diff --git a/internal/web/service/server.go b/internal/web/service/server.go index ec9360c2c..dd190d4b2 100644 --- a/internal/web/service/server.go +++ b/internal/web/service/server.go @@ -1132,6 +1132,40 @@ func (s *ServerService) GetLogs(count string, level string, syslog string) []str return lines } +// parseAccessLogFields extracts the structured fields from one Xray access-log +// line. Lines are attacker-influenced (a client's requested destination lands in +// the log verbatim) and may be truncated, so every positional lookup is length +// guarded: a malformed line yields a partial entry rather than panicking. +func parseAccessLogFields(line string) LogEntry { + var entry LogEntry + parts := strings.Fields(line) + + for i, part := range parts { + + if i == 0 && len(parts) > 1 { + dateTime, err := time.ParseInLocation("2006/01/02 15:04:05.999999", parts[0]+" "+parts[1], time.Local) + if err != nil { + continue + } + entry.DateTime = dateTime.UTC() + } + + if part == "from" && i+1 < len(parts) { + entry.FromAddress = strings.TrimLeft(parts[i+1], "/") + } else if part == "accepted" && i+1 < len(parts) { + entry.ToAddress = strings.TrimLeft(parts[i+1], "/") + } else if strings.HasPrefix(part, "[") { + entry.Inbound = part[1:] + } else if strings.HasSuffix(part, "]") { + entry.Outbound = part[:len(part)-1] + } else if part == "email:" && i+1 < len(parts) { + entry.Email = parts[i+1] + } + } + + return entry +} + func (s *ServerService) GetXrayLogs( count string, filter string, @@ -1176,31 +1210,7 @@ func (s *ServerService) GetXrayLogs( continue } - var entry LogEntry - parts := strings.Fields(line) - - for i, part := range parts { - - if i == 0 { - dateTime, err := time.ParseInLocation("2006/01/02 15:04:05.999999", parts[0]+" "+parts[1], time.Local) - if err != nil { - continue - } - entry.DateTime = dateTime.UTC() - } - - if part == "from" { - entry.FromAddress = strings.TrimLeft(parts[i+1], "/") - } else if part == "accepted" { - entry.ToAddress = strings.TrimLeft(parts[i+1], "/") - } else if strings.HasPrefix(part, "[") { - entry.Inbound = part[1:] - } else if strings.HasSuffix(part, "]") { - entry.Outbound = part[:len(part)-1] - } else if part == "email:" { - entry.Email = parts[i+1] - } - } + entry := parseAccessLogFields(line) if logEntryContains(line, freedoms) { if showDirect == "false" {