From 5bb87fd3d4142d606c3875641d7ca68aa8f1a5c4 Mon Sep 17 00:00:00 2001 From: Sanaei Date: Sat, 7 Feb 2026 22:34:06 +0100 Subject: [PATCH] fix : Uncontrolled data used in path expression Co-Authored-By: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- web/job/check_client_ip_job.go | 5 ++- web/service/server.go | 66 +++++++++++----------------------- 2 files changed, 22 insertions(+), 49 deletions(-) diff --git a/web/job/check_client_ip_job.go b/web/job/check_client_ip_job.go index 94486236..d3c1a1d1 100644 --- a/web/job/check_client_ip_job.go +++ b/web/job/check_client_ip_job.go @@ -3,7 +3,6 @@ package job import ( "bufio" "encoding/json" - "fmt" "io" "log" "os" @@ -388,7 +387,7 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun // disconnectClientTemporarily removes and re-adds a client to force disconnect old connections func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, clientEmail string, clients []model.Client) { var xrayAPI xray.XrayAPI - + // Get panel settings for API port db := database.GetDB() var apiPort int @@ -396,7 +395,7 @@ func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, c if err := db.Where("key = ?", "xrayApiPort").First(&apiPortSetting).Error; err == nil { apiPort, _ = strconv.Atoi(apiPortSetting.Value) } - + if apiPort == 0 { apiPort = 10085 // Default API port } diff --git a/web/service/server.go b/web/service/server.go index ec217e29..c78f7374 100644 --- a/web/service/server.go +++ b/web/service/server.go @@ -1056,35 +1056,23 @@ func (s *ServerService) IsValidGeofileName(filename string) bool { } func (s *ServerService) UpdateGeofile(fileName string) error { - files := []struct { + type geofileEntry struct { URL string FileName string - }{ - {"https://github.com/Loyalsoldier/v2ray-rules-dat/releases/latest/download/geoip.dat", "geoip.dat"}, - {"https://github.com/Loyalsoldier/v2ray-rules-dat/releases/latest/download/geosite.dat", "geosite.dat"}, - {"https://github.com/chocolate4u/Iran-v2ray-rules/releases/latest/download/geoip.dat", "geoip_IR.dat"}, - {"https://github.com/chocolate4u/Iran-v2ray-rules/releases/latest/download/geosite.dat", "geosite_IR.dat"}, - {"https://github.com/runetfreedom/russia-v2ray-rules-dat/releases/latest/download/geoip.dat", "geoip_RU.dat"}, - {"https://github.com/runetfreedom/russia-v2ray-rules-dat/releases/latest/download/geosite.dat", "geosite_RU.dat"}, + } + geofileAllowlist := map[string]geofileEntry{ + "geoip.dat": {"https://github.com/Loyalsoldier/v2ray-rules-dat/releases/latest/download/geoip.dat", "geoip.dat"}, + "geosite.dat": {"https://github.com/Loyalsoldier/v2ray-rules-dat/releases/latest/download/geosite.dat", "geosite.dat"}, + "geoip_IR.dat": {"https://github.com/chocolate4u/Iran-v2ray-rules/releases/latest/download/geoip.dat", "geoip_IR.dat"}, + "geosite_IR.dat": {"https://github.com/chocolate4u/Iran-v2ray-rules/releases/latest/download/geosite.dat", "geosite_IR.dat"}, + "geoip_RU.dat": {"https://github.com/runetfreedom/russia-v2ray-rules-dat/releases/latest/download/geoip.dat", "geoip_RU.dat"}, + "geosite_RU.dat": {"https://github.com/runetfreedom/russia-v2ray-rules-dat/releases/latest/download/geosite.dat", "geosite_RU.dat"}, } // Strict allowlist check to avoid writing uncontrolled files if fileName != "" { - // Use the centralized validation function - if !s.IsValidGeofileName(fileName) { - return common.NewErrorf("Invalid geofile name: contains unsafe path characters: %s", fileName) - } - - // Ensure the filename matches exactly one from our allowlist - isAllowed := false - for _, file := range files { - if fileName == file.FileName { - isAllowed = true - break - } - } - if !isAllowed { - return common.NewErrorf("Invalid geofile name: %s not in allowlist", fileName) + if _, ok := geofileAllowlist[fileName]; !ok { + return common.NewErrorf("Invalid geofile name: %q not in allowlist", fileName) } } @@ -1159,32 +1147,18 @@ func (s *ServerService) UpdateGeofile(fileName string) error { var errorMessages []string if fileName == "" { - for _, file := range files { - // Sanitize the filename from our allowlist as an extra precaution - destPath := filepath.Join(config.GetBinFolderPath(), filepath.Base(file.FileName)) - if err := downloadFile(file.URL, destPath); err != nil { - errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", file.FileName, err)) + // Download all geofiles + for _, entry := range geofileAllowlist { + destPath := filepath.Join(config.GetBinFolderPath(), entry.FileName) + if err := downloadFile(entry.URL, destPath); err != nil { + errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", entry.FileName, err)) } } } else { - // Use filepath.Base to ensure we only get the filename component, no path traversal - safeName := filepath.Base(fileName) - destPath := filepath.Join(config.GetBinFolderPath(), safeName) - - var fileURL string - for _, file := range files { - if file.FileName == fileName { - fileURL = file.URL - break - } - } - - if fileURL == "" { - errorMessages = append(errorMessages, fmt.Sprintf("File '%s' not found in the list of Geofiles", fileName)) - } else { - if err := downloadFile(fileURL, destPath); err != nil { - errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", fileName, err)) - } + entry := geofileAllowlist[fileName] + destPath := filepath.Join(config.GetBinFolderPath(), entry.FileName) + if err := downloadFile(entry.URL, destPath); err != nil { + errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", entry.FileName, err)) } }