Fixed SQL injections with ORDER BY and GROUP BY.

This commit is contained in:
Richard Knop 2016-05-06 19:12:36 +08:00
parent bf413d67d3
commit 5810691ee9
2 changed files with 56 additions and 6 deletions

View File

@ -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 {

43
sql_injection_test.go Normal file
View File

@ -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")
}
}