From 037aaa89732573f6147bdf5c4d7a80175f2fb167 Mon Sep 17 00:00:00 2001
From: Richard Knop <risoknop@gmail.com>
Date: Fri, 6 May 2016 19:08:02 +0800
Subject: [PATCH] Fixed SQL injections with ORDER BY and GROUP BY.

---
 example/example.go    | 103 ++++++++++++++++++++++++++++++++++++++++++
 search.go             |   6 +--
 sql_injection_test.go |  29 ++++++++++++
 3 files changed, 135 insertions(+), 3 deletions(-)
 create mode 100644 example/example.go
 create mode 100644 sql_injection_test.go

diff --git a/example/example.go b/example/example.go
new file mode 100644
index 00000000..741aa333
--- /dev/null
+++ b/example/example.go
@@ -0,0 +1,103 @@
+package main
+
+import (
+	"database/sql"
+
+	_ "github.com/go-sql-driver/mysql"
+	"github.com/jinzhu/gorm"
+	_ "github.com/lib/pq"
+	_ "github.com/mattn/go-sqlite3"
+)
+
+var db gorm.DB
+
+// Profile ...
+type Profile struct {
+	gorm.Model
+	Name string `sql:"type:varchar(40);not null"`
+}
+
+// User ...
+type User struct {
+	gorm.Model
+	Username     string `sql:"type:varchar(100);not null;unique"`
+	UserProfiles []*UserProfile
+}
+
+// UserProfile ...
+type UserProfile struct {
+	gorm.Model
+	ProfileID sql.NullInt64 `sql:"index;not null"`
+	UserID    sql.NullInt64 `sql:"index;not null"`
+	Profile   *Profile
+	User      *User
+	State     string `sql:"index;not null"`
+}
+
+func init() {
+	var err error
+	db, err = gorm.Open("sqlite3", ":memory:")
+	// db, err := gorm.Open("postgres", "user=username dbname=password sslmode=disable")
+	// db, err := gorm.Open("mysql", "user:password@/dbname?charset=utf8&parseTime=True")
+	if err != nil {
+		panic(err)
+	}
+	db.LogMode(true)
+
+	db.AutoMigrate(new(Profile), new(User), new(UserProfile))
+}
+
+func main() {
+	buyerProfile := &Profile{Name: "buyer"}
+	if err := db.Create(buyerProfile).Error; err != nil {
+		panic(err)
+	}
+	sellerProfile := &Profile{Name: "seller"}
+	if err := db.Create(sellerProfile).Error; err != nil {
+		panic(err)
+	}
+
+	user := &User{
+		Username: "username",
+		UserProfiles: []*UserProfile{
+			&UserProfile{
+				Profile: buyerProfile,
+				State:   "some_state",
+			},
+		},
+	}
+	if err := db.Create(user).Error; err != nil {
+		panic(err)
+	}
+
+	// Now let's update the user
+	tx := db.Begin()
+
+	user.Username = "username_edited"
+
+	user.UserProfiles = append(
+		user.UserProfiles,
+		&UserProfile{
+			Profile: sellerProfile,
+			State:   "some_state",
+		},
+	)
+
+	if err := tx.Model(user).Association("UserProfiles").Append(&UserProfile{
+		Profile: sellerProfile,
+		State:   "some_state",
+	}).Error; err != nil {
+		tx.Rollback() // rollback the transaction
+		panic(err)
+	}
+
+	// if err := tx.Save(user).Error; err != nil {
+	// 	tx.Rollback() // rollback the transaction
+	// 	panic(err)
+	// }
+
+	if err := tx.Commit().Error; err != nil {
+		tx.Rollback() // rollback the transaction
+		panic(err)
+	}
+}
diff --git a/search.go b/search.go
index 078bd429..3460fa3e 100644
--- a/search.go
+++ b/search.go
@@ -62,12 +62,12 @@ func (s *search) Assign(attrs ...interface{}) *search {
 func (s *search) Order(value string, reorder ...bool) *search {
 	if len(reorder) > 0 && reorder[0] {
 		if value != "" {
-			s.orders = []string{value}
+			s.orders = []string{s.db.dialect.Quote(value)}
 		} else {
 			s.orders = []string{}
 		}
 	} else if value != "" {
-		s.orders = append(s.orders, value)
+		s.orders = append(s.orders, s.db.dialect.Quote(value))
 	}
 	return s
 }
@@ -93,7 +93,7 @@ func (s *search) Offset(offset int) *search {
 }
 
 func (s *search) Group(query string) *search {
-	s.group = s.getInterfaceAsSQL(query)
+	s.group = s.db.dialect.Quote(s.getInterfaceAsSQL(query))
 	return s
 }
 
diff --git a/sql_injection_test.go b/sql_injection_test.go
new file mode 100644
index 00000000..4625f730
--- /dev/null
+++ b/sql_injection_test.go
@@ -0,0 +1,29 @@
+package gorm_test
+
+import "testing"
+
+func TestOrderSQLInjection(t *testing.T) {
+	DB.AutoMigrate(new(User))
+
+	DB.Save(&User{Name: "jinzhu"})
+
+	var users []*User
+	DB.Order("id;delete from users;commit;").Find(&users)
+
+	if len(users) != 1 {
+		t.Error("Seems like it's possible to use SQL injection with ORDER BY")
+	}
+}
+
+func TestGroupSQLInjection(t *testing.T) {
+	DB.AutoMigrate(new(User))
+
+	DB.Save(&User{Name: "jinzhu"})
+
+	var users []*User
+	DB.Group("name;delete from users;commit;").Find(&users)
+
+	if len(users) != 1 {
+		t.Error("Seems like it's possible to use SQL injection with GROUP BY")
+	}
+}