fix : Uncontrolled data used in path expression

Co-Authored-By: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

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))
 		}
 	}