SQL providers: make sure we don't exceed the allowed placeholders

Fixes #1415

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
This commit is contained in:
Nicola Murino 2023-09-12 19:16:54 +02:00
parent 9906caefd5
commit cf1cc25a48
No known key found for this signature in database
GPG key ID: 935D2952DEC4EECF
8 changed files with 207 additions and 66 deletions

12
go.mod
View file

@ -74,12 +74,12 @@ require (
golang.org/x/sys v0.12.0
golang.org/x/term v0.12.0
golang.org/x/time v0.3.0
google.golang.org/api v0.139.0
google.golang.org/api v0.140.0
gopkg.in/natefinch/lumberjack.v2 v2.2.1
)
require (
cloud.google.com/go v0.110.7 // indirect
cloud.google.com/go v0.110.8 // indirect
cloud.google.com/go/compute v1.23.0 // indirect
cloud.google.com/go/compute/metadata v0.2.3 // indirect
cloud.google.com/go/iam v1.1.2 // indirect
@ -135,7 +135,7 @@ require (
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.19 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
github.com/miekg/dns v1.1.55 // indirect
github.com/miekg/dns v1.1.56 // indirect
github.com/minio/sha256-simd v1.0.1 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
@ -162,9 +162,9 @@ require (
golang.org/x/tools v0.13.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.8 // indirect
google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d // indirect
google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832 // indirect
google.golang.org/grpc v1.58.0 // indirect
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect

26
go.sum
View file

@ -17,8 +17,8 @@ cloud.google.com/go v0.65.0/go.mod h1:O5N8zS7uWy9vkA9vayVHs65eM1ubvY4h553ofrNHOb
cloud.google.com/go v0.72.0/go.mod h1:M+5Vjvlc2wnp6tjzE102Dw08nGShTscUx2nZMufOKPI=
cloud.google.com/go v0.74.0/go.mod h1:VV1xSbzvo+9QJOxLDaJfTjx5e+MePCpCWwvftOeQmWk=
cloud.google.com/go v0.75.0/go.mod h1:VGuuCn7PG0dwsd5XPVm2Mm3wlh3EL55/79EKB6hlPTY=
cloud.google.com/go v0.110.7 h1:rJyC7nWRg2jWGZ4wSJ5nY65GTdYJkg0cd/uXb+ACI6o=
cloud.google.com/go v0.110.7/go.mod h1:+EYjdK8e5RME/VY/qLCAtuyALQ9q67dvuum8i+H5xsI=
cloud.google.com/go v0.110.8 h1:tyNdfIxjzaWctIiLYOTalaLKZ17SI44SKFW26QbOhME=
cloud.google.com/go v0.110.8/go.mod h1:Iz8AkXJf1qmxC3Oxoep8R1T36w8B92yU29PcBhHO5fk=
cloud.google.com/go/bigquery v1.0.1/go.mod h1:i/xbL2UlR5RvWAURpBYZTtm/cXjCha9lbfbpx4poX+o=
cloud.google.com/go/bigquery v1.3.0/go.mod h1:PjpwJnslEMmckchkHFfq+HTD2DmtT67aNFKH1/VBDHE=
cloud.google.com/go/bigquery v1.4.0/go.mod h1:S8dzgnTigyfTmLBfrtrhyYhwRxG72rYxvftPBK2Dvzc=
@ -359,8 +359,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zk
github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
github.com/mhale/smtpd v0.8.0 h1:5JvdsehCg33PQrZBvFyDMMUDQmvbzVpZgKob7eYBJc0=
github.com/mhale/smtpd v0.8.0/go.mod h1:MQl+y2hwIEQCXtNhe5+55n0GZOjSmeqORDIXbqUL3x4=
github.com/miekg/dns v1.1.55 h1:GoQ4hpsj0nFLYe+bWiCToyrBEJXkQfOOIvFGFy0lEgo=
github.com/miekg/dns v1.1.55/go.mod h1:uInx36IzPl7FYnDcMeVWxj9byh7DutNykX4G9Sj60FY=
github.com/miekg/dns v1.1.56 h1:5imZaSeoRNvpM9SzWNhEcP9QliKiz20/dA2QabIGVnE=
github.com/miekg/dns v1.1.56/go.mod h1:cRm6Oo2C8TY9ZS/TqsSrseAcncm74lfK5G+ikN2SWWY=
github.com/minio/sha256-simd v1.0.1 h1:6kaan5IFmwTNynnKKpDHe6FWHohJOHhCPchzK49dzMM=
github.com/minio/sha256-simd v1.0.1/go.mod h1:Pz6AKMiUdngCLpeTL/RJY1M9rUuPMYujV5xJjtbRSN8=
github.com/minio/sio v0.3.1 h1:d59r5RTHb1OsQaSl1EaTWurzMMDRLA5fgNmjzD4eVu4=
@ -460,8 +460,6 @@ github.com/unrolled/secure v1.13.0 h1:sdr3Phw2+f8Px8HE5sd1EHdj1aV3yUwed/uZXChLFs
github.com/unrolled/secure v1.13.0/go.mod h1:BmF5hyM6tXczk3MpQkFf1hpKSRqCyhqcbiQtiAF7+40=
github.com/wagslane/go-password-validator v0.3.0 h1:vfxOPzGHkz5S146HDpavl0cw1DSVP061Ry2PX0/ON6I=
github.com/wagslane/go-password-validator v0.3.0/go.mod h1:TI1XJ6T5fRdRnHqHt14pvy1tNVnrwe7m3/f1f2fDphQ=
github.com/wneessen/go-mail v0.4.0 h1:Oo4HLIV8My7G9JuZkoOX6eipXQD+ACvIqURYeIzUc88=
github.com/wneessen/go-mail v0.4.0/go.mod h1:zxOlafWCP/r6FEhAaRgH4IC1vg2YXxO0Nar9u0IScZ8=
github.com/wneessen/go-mail v0.4.1-0.20230823094700-0bd5390e370d h1:VBNB8NUpz3Acau8LFmpZ/nYT2TKfGjeOjqfrrr/G5T8=
github.com/wneessen/go-mail v0.4.1-0.20230823094700-0bd5390e370d/go.mod h1:zxOlafWCP/r6FEhAaRgH4IC1vg2YXxO0Nar9u0IScZ8=
github.com/yl2chen/cidranger v1.0.3-0.20210928021809-d1cb2c52f37a h1:XfF01GyP+0eWCaVp0y6rNN+kFp7pt9Da4UUYrJ5XPWA=
@ -745,8 +743,8 @@ google.golang.org/api v0.30.0/go.mod h1:QGmEvQ87FHZNiUVJkT14jQNYJ4ZJjdRF23ZXz513
google.golang.org/api v0.35.0/go.mod h1:/XrVsuzM0rZmrsbjJutiuftIzeuTQcEeaYcSk/mQ1dg=
google.golang.org/api v0.36.0/go.mod h1:+z5ficQTmoYpPn8LCUNVpK5I7hwkpjbcgqA7I34qYtE=
google.golang.org/api v0.40.0/go.mod h1:fYKFpnQN0DsDSKRVRcQSDQNtqWPfM9i+zNPxepjRCQ8=
google.golang.org/api v0.139.0 h1:A1TrCPgMmOiYu0AiNkvQIpIx+D8blHTDcJ5EogkP7LI=
google.golang.org/api v0.139.0/go.mod h1:CVagp6Eekz9CjGZ718Z+sloknzkDJE7Vc1Ckj9+viBk=
google.golang.org/api v0.140.0 h1:CaXNdYOH5oQQI7l6iKTHHiMTdxZca4/02hRg2U8c2hM=
google.golang.org/api v0.140.0/go.mod h1:aGbCiFgtwb2P6badchFbSBUurV6oR5d50Af4iNJtDdI=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
@ -792,12 +790,12 @@ google.golang.org/genproto v0.0.0-20201210142538-e3217bee35cc/go.mod h1:FWY/as6D
google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210108203827-ffc7fda8c3d7/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210226172003-ab064af71705/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d h1:VBu5YqKPv6XiJ199exd8Br+Aetz+o08F+PLMnwJQHAY=
google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d/go.mod h1:yZTlhN0tQnXo3h00fuXNCxJdLdIdnVFVBaRJ5LWBbw4=
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d h1:DoPTO70H+bcDXcd39vOqb2viZxgqeBeSGtZ55yZU4/Q=
google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d/go.mod h1:KjSP20unUpOx5kyQUFa7k4OJg0qeJ7DEZflGDu2p6Bk=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d h1:uvYuEyMHKNt+lT4K3bN6fGswmK8qSvcreM3BwjDh+y4=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d/go.mod h1:+Bk1OCOj40wS2hwAMA+aCW9ypzm63QTBBHp6lQ3p+9M=
google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832 h1:/30npZKtUjXqju7ZA2MsvpkGKD4mQFtf+zPnZasABjg=
google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832/go.mod h1:yZTlhN0tQnXo3h00fuXNCxJdLdIdnVFVBaRJ5LWBbw4=
google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832 h1:4E7rZzBdR5LmiZx6n47Dg4AjH8JLhMQWywsYqvXNLcs=
google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832/go.mod h1:KjSP20unUpOx5kyQUFa7k4OJg0qeJ7DEZflGDu2p6Bk=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832 h1:o4LtQxebKIJ4vkzyhtD2rfUNZ20Zf0ik5YVP5E7G7VE=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832/go.mod h1:+Bk1OCOj40wS2hwAMA+aCW9ypzm63QTBBHp6lQ3p+9M=
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=

View file

@ -1644,6 +1644,112 @@ func TestIPList(t *testing.T) {
}
}
func TestSQLPlaceholderLimits(t *testing.T) {
numGroups := 120
numUsers := 120
var groupMapping []sdk.GroupMapping
folder := vfs.BaseVirtualFolder{
Name: "testfolder",
MappedPath: filepath.Join(os.TempDir(), "folder"),
}
err := dataprovider.AddFolder(&folder, "", "", "")
assert.NoError(t, err)
for i := 0; i < numGroups; i++ {
group := dataprovider.Group{
BaseGroup: sdk.BaseGroup{
Name: fmt.Sprintf("testgroup%d", i),
},
UserSettings: dataprovider.GroupUserSettings{
BaseGroupUserSettings: sdk.BaseGroupUserSettings{
Permissions: map[string][]string{
fmt.Sprintf("/dir%d", i): {dataprovider.PermAny},
},
},
},
}
group.VirtualFolders = append(group.VirtualFolders, vfs.VirtualFolder{
BaseVirtualFolder: folder,
VirtualPath: "/vdir",
})
err := dataprovider.AddGroup(&group, "", "", "")
assert.NoError(t, err)
groupMapping = append(groupMapping, sdk.GroupMapping{
Name: group.Name,
Type: sdk.GroupTypeSecondary,
})
}
user := dataprovider.User{
BaseUser: sdk.BaseUser{
Username: "testusername",
HomeDir: filepath.Join(os.TempDir(), "testhome"),
Status: 1,
Permissions: map[string][]string{
"/": {dataprovider.PermAny},
},
},
Groups: groupMapping,
}
err = dataprovider.AddUser(&user, "", "", "")
assert.NoError(t, err)
users, err := dataprovider.GetUsersForQuotaCheck(map[string]bool{user.Username: true})
assert.NoError(t, err)
if assert.Len(t, users, 1) {
for i := 0; i < numGroups; i++ {
_, ok := users[0].Permissions[fmt.Sprintf("/dir%d", i)]
assert.True(t, ok)
}
}
err = dataprovider.DeleteUser(user.Username, "", "", "")
assert.NoError(t, err)
for i := 0; i < numUsers; i++ {
user := dataprovider.User{
BaseUser: sdk.BaseUser{
Username: fmt.Sprintf("testusername%d", i),
HomeDir: filepath.Join(os.TempDir()),
Status: 1,
Permissions: map[string][]string{
"/": {dataprovider.PermAny},
},
},
Groups: []sdk.GroupMapping{
{
Name: "testgroup0",
Type: sdk.GroupTypePrimary,
},
},
}
err := dataprovider.AddUser(&user, "", "", "")
assert.NoError(t, err)
}
time.Sleep(100 * time.Millisecond)
err = dataprovider.DeleteFolder(folder.Name, "", "", "")
assert.NoError(t, err)
for i := 0; i < numUsers; i++ {
username := fmt.Sprintf("testusername%d", i)
user, err := dataprovider.UserExists(username, "")
assert.NoError(t, err)
assert.Greater(t, user.UpdatedAt, user.CreatedAt)
err = dataprovider.DeleteUser(username, "", "", "")
assert.NoError(t, err)
}
for i := 0; i < numGroups; i++ {
groupName := fmt.Sprintf("testgroup%d", i)
err = dataprovider.DeleteGroup(groupName, "", "", "")
assert.NoError(t, err)
}
}
func BenchmarkBcryptHashing(b *testing.B) {
bcryptPassword := "bcryptpassword"
for i := 0; i < b.N; i++ {

View file

@ -592,7 +592,7 @@ func TestDataTransferExceeded(t *testing.T) {
func TestGetUsersForQuotaCheck(t *testing.T) {
usersToFetch := make(map[string]bool)
for i := 0; i < 50; i++ {
for i := 0; i < 70; i++ {
usersToFetch[fmt.Sprintf("user%v", i)] = i%2 == 0
}
@ -600,7 +600,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, users, 0)
for i := 0; i < 40; i++ {
for i := 0; i < 60; i++ {
user := dataprovider.User{
BaseUser: sdk.BaseUser{
Username: fmt.Sprintf("user%v", i),
@ -631,7 +631,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
users, err = dataprovider.GetUsersForQuotaCheck(usersToFetch)
assert.NoError(t, err)
assert.Len(t, users, 40)
assert.Len(t, users, 60)
for _, user := range users {
userIdxStr := strings.Replace(user.Username, "user", "", 1)
@ -655,7 +655,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
assert.Equal(t, int64(0), total)
}
for i := 0; i < 40; i++ {
for i := 0; i < 60; i++ {
err = dataprovider.DeleteUser(fmt.Sprintf("user%v", i), "", "", "")
assert.NoError(t, err)
err = dataprovider.DeleteFolder(fmt.Sprintf("f%v", i), "", "", "")

View file

@ -958,16 +958,23 @@ func sqlCommonGetUsersInGroups(names []string, dbHandle sqlQuerier) ([]string, e
if len(names) == 0 {
return nil, nil
}
maxNames := len(sqlPlaceholders)
usernames := make([]string, 0, len(names))
ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
defer cancel()
q := getUsersInGroupsQuery(len(names))
args := make([]any, 0, len(names))
for _, name := range names {
for len(names) > 0 {
if maxNames > len(names) {
maxNames = len(names)
}
q := getUsersInGroupsQuery(maxNames)
args := make([]any, 0, maxNames)
for _, name := range names[:maxNames] {
args = append(args, name)
}
usernames := make([]string, 0, len(names))
rows, err := dbHandle.QueryContext(ctx, q, args...)
if err != nil {
return nil, err
@ -982,22 +989,33 @@ func sqlCommonGetUsersInGroups(names []string, dbHandle sqlQuerier) ([]string, e
}
usernames = append(usernames, username)
}
return usernames, rows.Err()
err = rows.Err()
if err != nil {
return usernames, err
}
names = names[maxNames:]
}
return usernames, nil
}
func sqlCommonGetGroupsWithNames(names []string, dbHandle sqlQuerier) ([]Group, error) {
if len(names) == 0 {
return nil, nil
}
maxNames := len(sqlPlaceholders)
groups := make([]Group, 0, len(names))
for len(names) > 0 {
if maxNames > len(names) {
maxNames = len(names)
}
ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
defer cancel()
q := getGroupsWithNamesQuery(len(names))
args := make([]any, 0, len(names))
for _, name := range names {
q := getGroupsWithNamesQuery(maxNames)
args := make([]any, 0, maxNames)
for _, name := range names[:maxNames] {
args = append(args, name)
}
groups := make([]Group, 0, len(names))
rows, err := dbHandle.QueryContext(ctx, q, args...)
if err != nil {
return groups, err
@ -1015,6 +1033,11 @@ func sqlCommonGetGroupsWithNames(names []string, dbHandle sqlQuerier) ([]Group,
if err != nil {
return groups, err
}
names = names[maxNames:]
}
ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
defer cancel()
return getGroupsWithVirtualFolders(ctx, groups, dbHandle)
}
@ -1535,6 +1558,9 @@ func sqlCommonGetRecentlyUpdatedUsers(after int64, dbHandle sqlQuerier) ([]User,
}
}
groupNames = util.RemoveDuplicates(groupNames, false)
if len(groupNames) == 0 {
return users, nil
}
groups, err := sqlCommonGetGroupsWithNames(groupNames, dbHandle)
if err != nil {
return users, err
@ -1553,15 +1579,23 @@ func sqlCommonGetRecentlyUpdatedUsers(after int64, dbHandle sqlQuerier) ([]User,
return users, nil
}
func sqlGetMaxUsersForQuotaCheckRange() int {
maxUsers := 50
if maxUsers > len(sqlPlaceholders) {
maxUsers = len(sqlPlaceholders)
}
return maxUsers
}
func sqlCommonGetUsersForQuotaCheck(toFetch map[string]bool, dbHandle sqlQuerier) ([]User, error) {
users := make([]User, 0, 30)
maxUsers := sqlGetMaxUsersForQuotaCheckRange()
users := make([]User, 0, maxUsers)
usernames := make([]string, 0, len(toFetch))
for k := range toFetch {
usernames = append(usernames, k)
}
maxUsers := 30
for len(usernames) > 0 {
if maxUsers > len(usernames) {
maxUsers = len(usernames)

View file

@ -42,7 +42,7 @@ const (
func getSQLPlaceholders() []string {
var placeholders []string
for i := 1; i <= 50; i++ {
for i := 1; i <= 100; i++ {
if config.Driver == PGSQLDataProviderName || config.Driver == CockroachDataProviderName {
placeholders = append(placeholders, fmt.Sprintf("$%d", i))
} else {
@ -400,7 +400,7 @@ func getUsersInGroupsQuery(numArgs int) string {
} else {
sb.WriteString("('')")
}
return fmt.Sprintf(`SELECT username FROM %s WHERE id IN (SELECT user_id from %s WHERE group_id IN (SELECT id FROM %s WHERE name IN (%s)))`,
return fmt.Sprintf(`SELECT username FROM %s WHERE id IN (SELECT user_id from %s WHERE group_id IN (SELECT id FROM %s WHERE name IN %s))`,
sqlTableUsers, sqlTableUsersGroupsMapping, getSQLQuotedName(sqlTableGroups), sb.String())
}

View file

@ -1832,6 +1832,9 @@ func (u *User) mergeVirtualFolders(group *Group, groupType int, replacer *string
}
func (u *User) mergePermissions(group *Group, groupType int, replacer *strings.Replacer) {
if u.Permissions == nil {
u.Permissions = make(map[string][]string)
}
for k, v := range group.UserSettings.Permissions {
if k == "/" {
if groupType == sdk.GroupTypePrimary {

View file

@ -122,7 +122,7 @@ const (
pageForgotPwdTitle = "SFTPGo Admin - Forgot password"
pageResetPwdTitle = "SFTPGo Admin - Reset password"
pageSetupTitle = "Create first admin user"
defaultQueryLimit = 500
defaultQueryLimit = 1000
inversePatternType = "inverse"
)
@ -4169,7 +4169,7 @@ func (s *httpdServer) handleOAuth2TokenRedirect(w http.ResponseWriter, r *http.R
errTxt := "the OAuth2 provider returned an empty token. " +
"Some providers only return the token when the user first authorizes. " +
"If you have already registered SFTPGo with this user in the past, revoke access and try again. " +
"This way you will invalidate the previous token."
"This way you will invalidate the previous token"
s.renderMessagePage(w, r, errorTitle, "Unable to get token:", http.StatusBadRequest, errors.New(errTxt), "")
return
}