CVE-2024-22198: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: Nginx-UI is a web interface to manage Nginx configurations. It is vulnerable to arbitrary command execution by abusing the configuration settings. The `Home > Preference` page exposes a list of system settings such as `Run Mode`, `Jwt Secret`, `Node Secret` and `Terminal Start Command`. While the UI doesn't allow users to modify the `Terminal Start Command` setting, it is possible to do so by sending a request to the API. This issue may lead to authenticated remote code execution, privilege escalation, and information disclosure. This vulnerability has been patched in version 2.0.0.beta.9.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2024-01-11T20:15Z
Repository: https://github.com/0xJacky/nginx-ui
Language: Go
Files changed: api/system/settings.go, settings/nginx.go, settings/server.go
Commit date: 2023-12-20 03:49:41 +0900
Commit message: fix: add protected fields to settings to mitigate high-severity vulnerability

Credits to @jorgectf for the advisories.

Diff: diff --git a/api/system/settings.go b/api/system/settings.go
index 6908111..e87252b 100644
--- a/api/system/settings.go
+++ b/api/system/settings.go
@@ -1,42 +1,57 @@
 package system
 
 import (
-    "github.com/0xJacky/Nginx-UI/api"
-    "github.com/0xJacky/Nginx-UI/settings"
-    "github.com/gin-gonic/gin"
-    "net/http"
+	"github.com/0xJacky/Nginx-UI/api"
+	"github.com/0xJacky/Nginx-UI/settings"
+	"github.com/gin-gonic/gin"
+	"net/http"
+	"reflect"
 )
 
 func GetSettings(c *gin.Context) {
-    c.JSON(http.StatusOK, gin.H{
-        "server": settings.ServerSettings,
-        "nginx":  settings.NginxSettings,
-        "openai": settings.OpenAISettings,
-    })
+	c.JSON(http.StatusOK, gin.H{
+		"server": settings.ServerSettings,
+		"nginx":  settings.NginxSettings,
+		"openai": settings.OpenAISettings,
+	})
 }
 
 func SaveSettings(c *gin.Context) {
-    var json struct {
-        Server settings.Server `json:"server"`
-        Nginx  settings.Nginx  `json:"nginx"`
-        Openai settings.OpenAI `json:"openai"`
-    }
-
-    if !api.BindAndValid(c, &json) {
-        return
-    }
-
-    settings.ServerSettings = json.Server
-    settings.NginxSettings = json.Nginx
-    settings.OpenAISettings = json.Openai
-
-    settings.ReflectFrom()
-
-    err := settings.Save()
-    if err != nil {
-        api.ErrHandler(c, err)
-        return
-    }
+	var json struct {
+		Server settings.Server `json:"server"`
+		Nginx  settings.Nginx  `json:"nginx"`
+		Openai settings.OpenAI `json:"openai"`
+	}
+
+	if !api.BindAndValid(c, &json) {
+		return
+	}
+
+	// todo: omit protected fields when binding
+	fillSettings(&settings.ServerSettings, &json.Server)
+	fillSettings(&settings.NginxSettings, &json.Nginx)
+	fillSettings(&settings.OpenAISettings, &json.Openai)
+
+	settings.ReflectFrom()
+
+	err := settings.Save()
+	if err != nil {
+		api.ErrHandler(c, err)
+		return
+	}
+
+	GetSettings(c)
+}
 
-    GetSettings(c)
+func fillSettings(targetSettings interface{}, newSettings interface{}) {
+	s := reflect.TypeOf(targetSettings).Elem()
+	vt := reflect.ValueOf(targetSettings).Elem()
+	vn := reflect.ValueOf(newSettings).Elem()
+
+	// copy the values from new to target settings if it is not protected
+	for i := 0; i < s.NumField(); i++ {
+		if s.Field(i).Tag.Get("protected") != "true" {
+			vt.Field(i).Set(vn.Field(i))
+		}
+	}
 }
diff --git a/settings/nginx.go b/settings/nginx.go
index 4c1a5fd..3727d34 100644
--- a/settings/nginx.go
+++ b/settings/nginx.go
@@ -1,15 +1,15 @@
 package settings
 
 type Nginx struct {
 	AccessLogPath string `json:"access_log_path"`
 	ErrorLogPath  string `json:"error_log_path"`
-	ConfigDir     string `json:"config_dir"`
-	PIDPath       string `json:"pid_path"`
-	TestConfigCmd string `json:"test_config_cmd"`
-	ReloadCmd     string `json:"reload_cmd"`
-	RestartCmd    string `json:"restart_cmd"`
+	ConfigDir     string `json:"config_dir" protected:"true"`
+	PIDPath       string `json:"pid_path" protected:"true"`
+	TestConfigCmd string `json:"test_config_cmd" protected:"true"`
+	ReloadCmd     string `json:"reload_cmd" protected:"true"`
+	RestartCmd    string `json:"restart_cmd" protected:"true"`
 }
 
 var NginxSettings = Nginx{
 	AccessLogPath: "",
 	ErrorLogPath:  "",
diff --git a/settings/server.go b/settings/server.go
index 2d7785e..97d15bd 100644
--- a/settings/server.go
+++ b/settings/server.go
@@ -1,20 +1,20 @@
 package settings
 
 type Server struct {
-	HttpHost          string `json:"http_host"`
-	HttpPort          string `json:"http_port"`
-	RunMode           string `json:"run_mode"`
-	JwtSecret         string `json:"jwt_secret"`
-	NodeSecret        string `json:"node_secret"`
+	HttpHost          string `json:"http_host" protected:"true"`
+	HttpPort          string `json:"http_port" protected:"true"`
+	RunMode           string `json:"run_mode" protected:"true"`
+	JwtSecret         string `json:"jwt_secret" protected:"true"`
+	NodeSecret        string `json:"node_secret" protected:"true"`
 	HTTPChallengePort string `json:"http_challenge_port"`
-	Email             string `json:"email"`
-	Database          string `json:"database"`
-	StartCmd          string `json:"start_cmd"`
+	Email             string `json:"email" protected:"true"`
+	Database          string `json:"database" protected:"true"`
+	StartCmd          string `json:"start_cmd" protected:"true"`
 	CADir             string `json:"ca_dir"`
-	Demo              bool   `json:"demo"`
-	PageSize          int    `json:"page_size"`
+	Demo              bool   `json:"demo" protected:"true"`
+	PageSize          int    `json:"page_size" protected:"true"`
 	GithubProxy       string `json:"github_proxy"`
 }
 
 var ServerSettings = Server{
 	HttpHost:          "0.0.0.0",

Vulnerable code:     "github.com/0xJacky/Nginx-UI/api"
    "github.com/0xJacky/Nginx-UI/settings"
    "github.com/gin-gonic/gin"
    "net/http"
    c.JSON(http.StatusOK, gin.H{
        "server": settings.ServerSettings,
        "nginx":  settings.NginxSettings,
        "openai": settings.OpenAISettings,
    })
    var json struct {
        Server settings.Server `json:"server"`
        Nginx  settings.Nginx  `json:"nginx"`
        Openai settings.OpenAI `json:"openai"`
    }

    if !api.BindAndValid(c, &json) {
        return
    }

    settings.ServerSettings = json.Server
    settings.NginxSettings = json.Nginx
    settings.OpenAISettings = json.Openai

    settings.ReflectFrom()

    err := settings.Save()
    if err != nil {
        api.ErrHandler(c, err)
        return
    }
    GetSettings(c)
	ConfigDir     string `json:"config_dir"`
	PIDPath       string `json:"pid_path"`
	TestConfigCmd string `json:"test_config_cmd"`
	ReloadCmd     string `json:"reload_cmd"`
	RestartCmd    string `json:"restart_cmd"`
	HttpHost          string `json:"http_host"`
	HttpPort          string `json:"http_port"`
	RunMode           string `json:"run_mode"`
	JwtSecret         string `json:"jwt_secret"`
	NodeSecret        string `json:"node_secret"`
	Email             string `json:"email"`
	Database          string `json:"database"`
	StartCmd          string `json:"start_cmd"`
	Demo              bool   `json:"demo"`
	PageSize          int    `json:"page_size"`
Fixed code: 	"github.com/0xJacky/Nginx-UI/api"
	"github.com/0xJacky/Nginx-UI/settings"
	"github.com/gin-gonic/gin"
	"net/http"
	"reflect"
	c.JSON(http.StatusOK, gin.H{
		"server": settings.ServerSettings,
		"nginx":  settings.NginxSettings,
		"openai": settings.OpenAISettings,
	})
	var json struct {
		Server settings.Server `json:"server"`
		Nginx  settings.Nginx  `json:"nginx"`
		Openai settings.OpenAI `json:"openai"`
	}

	if !api.BindAndValid(c, &json) {
		return
	}

	// todo: omit protected fields when binding
	fillSettings(&settings.ServerSettings, &json.Server)
	fillSettings(&settings.NginxSettings, &json.Nginx)
	fillSettings(&settings.OpenAISettings, &json.Openai)

	settings.ReflectFrom()

	err := settings.Save()
	if err != nil {
		api.ErrHandler(c, err)
		return
	}

	GetSettings(c)
}
func fillSettings(targetSettings interface{}, newSettings interface{}) {
	s := reflect.TypeOf(targetSettings).Elem()
	vt := reflect.ValueOf(targetSettings).Elem()
	vn := reflect.ValueOf(newSettings).Elem()

	// copy the values from new to target settings if it is not protected
	for i := 0; i < s.NumField(); i++ {
		if s.Field(i).Tag.Get("protected") != "true" {
			vt.Field(i).Set(vn.Field(i))
		}
	}
	ConfigDir     string `json:"config_dir" protected:"true"`
	PIDPath       string `json:"pid_path" protected:"true"`
	TestConfigCmd string `json:"test_config_cmd" protected:"true"`
	ReloadCmd     string `json:"reload_cmd" protected:"true"`
	RestartCmd    string `json:"restart_cmd" protected:"true"`
	HttpHost          string `json:"http_host" protected:"true"`
	HttpPort          string `json:"http_port" protected:"true"`
	RunMode           string `json:"run_mode" protected:"true"`
	JwtSecret         string `json:"jwt_secret" protected:"true"`
	NodeSecret        string `json:"node_secret" protected:"true"`
	Email             string `json:"email" protected:"true"`
	Database          string `json:"database" protected:"true"`
	StartCmd          string `json:"start_cmd" protected:"true"`
	Demo              bool   `json:"demo" protected:"true"`
	PageSize          int    `json:"page_size" protected:"true"`