diff --git a/scope.go b/scope.go index 0116da08..a9871726 100644 --- a/scope.go +++ b/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 { diff --git a/sql_injection_test.go b/sql_injection_test.go new file mode 100644 index 00000000..306ce4a3 --- /dev/null +++ b/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") + } +}