mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2025-10-13 11:39:13 +00:00
security fix: Uncontrolled data used in path expression
This commit is contained in:
parent
806ecbd7c5
commit
55f1d72af5
2 changed files with 55 additions and 7 deletions
|
@ -138,6 +138,14 @@ func (a *ServerController) installXray(c *gin.Context) {
|
||||||
// updateGeofile updates the specified geo file for Xray.
|
// updateGeofile updates the specified geo file for Xray.
|
||||||
func (a *ServerController) updateGeofile(c *gin.Context) {
|
func (a *ServerController) updateGeofile(c *gin.Context) {
|
||||||
fileName := c.Param("fileName")
|
fileName := c.Param("fileName")
|
||||||
|
|
||||||
|
// Validate the filename for security (prevent path traversal attacks)
|
||||||
|
if fileName != "" && !a.serverService.IsValidGeofileName(fileName) {
|
||||||
|
jsonMsg(c, I18nWeb(c, "pages.index.geofileUpdatePopover"),
|
||||||
|
fmt.Errorf("invalid filename: contains unsafe characters or path traversal patterns"))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
err := a.serverService.UpdateGeofile(fileName)
|
err := a.serverService.UpdateGeofile(fileName)
|
||||||
jsonMsg(c, I18nWeb(c, "pages.index.geofileUpdatePopover"), err)
|
jsonMsg(c, I18nWeb(c, "pages.index.geofileUpdatePopover"), err)
|
||||||
}
|
}
|
||||||
|
|
|
@ -13,6 +13,7 @@ import (
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
"runtime"
|
"runtime"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
@ -996,6 +997,35 @@ func (s *ServerService) ImportDB(file multipart.File) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// IsValidGeofileName validates that the filename is safe for geofile operations.
|
||||||
|
// It checks for path traversal attempts and ensures the filename contains only safe characters.
|
||||||
|
func (s *ServerService) IsValidGeofileName(filename string) bool {
|
||||||
|
if filename == "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check for path traversal attempts
|
||||||
|
if strings.Contains(filename, "..") {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check for path separators (both forward and backward slash)
|
||||||
|
if strings.ContainsAny(filename, `/\`) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check for absolute path indicators
|
||||||
|
if filepath.IsAbs(filename) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Additional security: only allow alphanumeric, dots, underscores, and hyphens
|
||||||
|
// This is stricter than the general filename regex
|
||||||
|
validGeofilePattern := `^[a-zA-Z0-9._-]+\.dat$`
|
||||||
|
matched, _ := regexp.MatchString(validGeofilePattern, filename)
|
||||||
|
return matched
|
||||||
|
}
|
||||||
|
|
||||||
func (s *ServerService) UpdateGeofile(fileName string) error {
|
func (s *ServerService) UpdateGeofile(fileName string) error {
|
||||||
files := []struct {
|
files := []struct {
|
||||||
URL string
|
URL string
|
||||||
|
@ -1008,8 +1038,15 @@ func (s *ServerService) UpdateGeofile(fileName string) error {
|
||||||
{"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/geoip.dat", "geoip_RU.dat"},
|
||||||
{"https://github.com/runetfreedom/russia-v2ray-rules-dat/releases/latest/download/geosite.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
|
// Strict allowlist check to avoid writing uncontrolled files
|
||||||
if fileName != "" {
|
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
|
isAllowed := false
|
||||||
for _, file := range files {
|
for _, file := range files {
|
||||||
if fileName == file.FileName {
|
if fileName == file.FileName {
|
||||||
|
@ -1018,7 +1055,7 @@ func (s *ServerService) UpdateGeofile(fileName string) error {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if !isAllowed {
|
if !isAllowed {
|
||||||
return common.NewErrorf("Invalid geofile name: %s", fileName)
|
return common.NewErrorf("Invalid geofile name: %s not in allowlist", fileName)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
downloadFile := func(url, destPath string) error {
|
downloadFile := func(url, destPath string) error {
|
||||||
|
@ -1046,14 +1083,17 @@ func (s *ServerService) UpdateGeofile(fileName string) error {
|
||||||
|
|
||||||
if fileName == "" {
|
if fileName == "" {
|
||||||
for _, file := range files {
|
for _, file := range files {
|
||||||
destPath := fmt.Sprintf("%s/%s", config.GetBinFolderPath(), file.FileName)
|
// 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 {
|
if err := downloadFile(file.URL, destPath); err != nil {
|
||||||
errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", file.FileName, err))
|
errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", file.FileName, err))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
destPath := fmt.Sprintf("%s/%s", config.GetBinFolderPath(), fileName)
|
// 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
|
var fileURL string
|
||||||
for _, file := range files {
|
for _, file := range files {
|
||||||
|
@ -1065,10 +1105,10 @@ func (s *ServerService) UpdateGeofile(fileName string) error {
|
||||||
|
|
||||||
if fileURL == "" {
|
if fileURL == "" {
|
||||||
errorMessages = append(errorMessages, fmt.Sprintf("File '%s' not found in the list of Geofiles", fileName))
|
errorMessages = append(errorMessages, fmt.Sprintf("File '%s' not found in the list of Geofiles", fileName))
|
||||||
}
|
} else {
|
||||||
|
if err := downloadFile(fileURL, destPath); err != nil {
|
||||||
if err := downloadFile(fileURL, destPath); err != nil {
|
errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", fileName, err))
|
||||||
errorMessages = append(errorMessages, fmt.Sprintf("Error downloading Geofile '%s': %v", fileName, err))
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue