Merge 5810691ee9fef7360d85420d41fe8e214c3374a1 into 57c72125b3087e1ed02c16f23a1f381cde3aaf2c

scope.go

@@ -723,15 +723,22 @@ func (scope *Scope) selectSQL() string {
 	return scope.buildSelectQuery(scope.Search.selects)
 }
 
+// only match string like `name`, `users.name`, `name ASC`, `users.name desc`
+var orderByRegexp = regexp.MustCompile("^\"?[a-zA-Z0-9]+\"?(\\.\"?[a-zA-Z0-9]+\"?)?(?i)( (asc|desc))?$")
+
 func (scope *Scope) orderSQL() string {
-	if len(scope.Search.orders) == 0 || scope.Search.countingQuery {
+	var orders = []string{}
+	for _, order := range scope.Search.orders {
+		if !orderByRegexp.MatchString(order) {
+			continue
+		}
+		orders = append(orders, scope.quoteIfPossible(order))
+	}
+
+	if len(orders) == 0 || scope.Search.countingQuery {
 		return ""
 	}
 
-	var orders []string
-	for _, order := range scope.Search.orders {
-		orders = append(orders, scope.quoteIfPossible(order))
-	}
 	return " ORDER BY " + strings.Join(orders, ",")
 }
 
@@ -743,7 +750,7 @@ func (scope *Scope) groupSQL() string {
 	if len(scope.Search.group) == 0 {
 		return ""
 	}
-	return " GROUP BY " + scope.Search.group
+	return " GROUP BY " + scope.Quote(scope.Search.group)
 }
 
 func (scope *Scope) havingSQL() string {

sql_injection_test.go

@@ -0,0 +1,43 @@
+package gorm_test
+
+import "testing"
+
+func TestOrderSQLInjection(t *testing.T) {
+	DB.AutoMigrate(new(User))
+
+	testUser := &User{Name: "jinzhu"}
+	DB.Save(testUser)
+
+	var countBefore int
+	DB.Model(new(User)).Count(&countBefore)
+
+	var users []*User
+	DB.Order("id;delete from users;commit;").Find(&users)
+
+	var countAfter int
+	DB.Model(new(User)).Count(&countAfter)
+
+	if countAfter != countBefore {
+		t.Error("Seems like it's possible to use SQL injection with ORDER BY")
+	}
+}
+
+func TestGroupSQLInjection(t *testing.T) {
+	DB.AutoMigrate(new(User))
+
+	testUser := &User{Name: "jinzhu"}
+	DB.Save(testUser)
+
+	var countBefore int
+	DB.Model(new(User)).Count(&countBefore)
+
+	var users []*User
+	DB.Group("name;delete from users;commit;").Find(&users)
+
+	var countAfter int
+	DB.Model(new(User)).Count(&countAfter)
+
+	if countAfter != countBefore {
+		t.Error("Seems like it's possible to use SQL injection with GROUP BY")
+	}
+}