CVE-2024-22196: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Nginx-UI is an online statistics for Server Indicators?? Monitor CPU usage, memory usage, load average, and disk usage in real-time. This issue may lead to information disclosure. By using `DefaultQuery`, the `"desc"` and `"id"` values are used as default values if the query parameters are not set. Thus, the `order` and `sort_by` query parameter are user-controlled and are being appended to the `order` variable without any sanitization. This issue has been patched in version 2.0.0.beta.9.


CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 6.5
Published: 2024-01-11T20:15Z
Repository: https://github.com/0xJacky/nginx-ui
Language: Go
Files changed: api/cosy/sort.go, model/model.go
Commit date: 2023-12-20 04:52:02 +0900
Commit message: fix: ensure the list sort query is validated to prevent SQL injection

Credits to @jorgectf for the advisories.

Diff: diff --git a/api/cosy/sort.go b/api/cosy/sort.go
index d1894dd..ad612bd 100644
--- a/api/cosy/sort.go
+++ b/api/cosy/sort.go
@@ -1,30 +1,42 @@
 package cosy
 
 import (
 	"fmt"
+	"github.com/0xJacky/Nginx-UI/internal/logger"
 	"github.com/gin-gonic/gin"
 	"gorm.io/gorm"
+	"gorm.io/gorm/schema"
+	"sync"
 )
 
 func (c *Ctx[T]) SortOrder() func(db *gorm.DB) *gorm.DB {
 	return func(db *gorm.DB) *gorm.DB {
 		sort := c.ctx.DefaultQuery("order", "desc")
-		order := fmt.Sprintf("%s %s", DefaultQuery(c.ctx, "sort_by", c.itemKey), sort)
-		return db.Order(order)
+		if sort != "desc" && sort != "asc" {
+			sort = "desc"
+		}
+
+		// check if the order field is valid
+		// todo: maybe we can use more generic way to check if the sort_by is valid
+		order := DefaultQuery(c.ctx, "sort_by", c.itemKey)
+		s, _ := schema.Parse(c.Model, &sync.Map{}, schema.NamingStrategy{})
+		if _, ok := s.FieldsByDBName[order]; ok {
+			order = fmt.Sprintf("%s %s", order, sort)
+			return db.Order(order)
+		} else {
+			logger.Error("invalid order field:", order)
+		}
+
+		return db
 	}
 }
 
 func (c *Ctx[T]) OrderAndPaginate() func(db *gorm.DB) *gorm.DB {
 	return func(db *gorm.DB) *gorm.DB {
-		sort := c.ctx.DefaultQuery("order", "desc")
-
-		order := fmt.Sprintf("%s %s", DefaultQuery(c.ctx, "sort_by", c.itemKey), sort)
-		db = db.Order(order)
-
+		db = c.SortOrder()(db)
 		_, offset, pageSize := GetPagingParams(c.ctx)
-
 		return db.Offset(offset).Limit(pageSize)
 	}
 }
 
 func DefaultValue(c *gin.Context, key string, defaultValue any) any {
diff --git a/model/model.go b/model/model.go
index 54ae22d..b1ecbe9 100644
--- a/model/model.go
+++ b/model/model.go
@@ -8,12 +8,14 @@ import (
 	"github.com/spf13/cast"
 	"gorm.io/driver/sqlite"
 	"gorm.io/gen"
 	"gorm.io/gorm"
 	gormlogger "gorm.io/gorm/logger"
+	"gorm.io/gorm/schema"
 	"path"
 	"strings"
+	"sync"
 	"time"
 )
 
 var db *gorm.DB
 
@@ -98,13 +100,23 @@ func SortOrder(c *gin.Context) func(db *gorm.DB) *gorm.DB {
 }
 
 func OrderAndPaginate(c *gin.Context) func(db *gorm.DB) *gorm.DB {
 	return func(db *gorm.DB) *gorm.DB {
 		sort := c.DefaultQuery("order", "desc")
+		if sort != "desc" && sort != "asc" {
+			sort = "desc"
+		}
 
-		order := fmt.Sprintf("`%s` %s", DefaultQuery(c, "sort_by", "id"), sort)
-		db = db.Order(order)
+		// check if the order field is valid
+		order := c.DefaultQuery("sort_by", "id")
+		s, _ := schema.Parse(db.Model, &sync.Map{}, schema.NamingStrategy{})
+		if _, ok := s.FieldsByName[order]; ok {
+			order = fmt.Sprintf("%s %s", order, sort)
+			db = db.Order(order)
+		} else {
+			logger.Error("invalid order field: ", order)
+		}
 
 		page := cast.ToInt(c.Query("page"))
 		if page == 0 {
 			page = 1
 		}

Vulnerable code: 		order := fmt.Sprintf("%s %s", DefaultQuery(c.ctx, "sort_by", c.itemKey), sort)
		return db.Order(order)
		sort := c.ctx.DefaultQuery("order", "desc")

		order := fmt.Sprintf("%s %s", DefaultQuery(c.ctx, "sort_by", c.itemKey), sort)
		db = db.Order(order)


		order := fmt.Sprintf("`%s` %s", DefaultQuery(c, "sort_by", "id"), sort)
		db = db.Order(order)
Fixed code: 	"github.com/0xJacky/Nginx-UI/internal/logger"
	"gorm.io/gorm/schema"
	"sync"
		if sort != "desc" && sort != "asc" {
			sort = "desc"
		}

		// check if the order field is valid
		// todo: maybe we can use more generic way to check if the sort_by is valid
		order := DefaultQuery(c.ctx, "sort_by", c.itemKey)
		s, _ := schema.Parse(c.Model, &sync.Map{}, schema.NamingStrategy{})
		if _, ok := s.FieldsByDBName[order]; ok {
			order = fmt.Sprintf("%s %s", order, sort)
			return db.Order(order)
		} else {
			logger.Error("invalid order field:", order)
		}

		return db
		db = c.SortOrder()(db)
	"gorm.io/gorm/schema"
	"sync"
		if sort != "desc" && sort != "asc" {
			sort = "desc"
		}
		// check if the order field is valid
		order := c.DefaultQuery("sort_by", "id")
		s, _ := schema.Parse(db.Model, &sync.Map{}, schema.NamingStrategy{})
		if _, ok := s.FieldsByName[order]; ok {
			order = fmt.Sprintf("%s %s", order, sort)
			db = db.Order(order)
		} else {
			logger.Error("invalid order field: ", order)
		}