From 89553864f5fa92beaa37a12e489064b1358d9880 Mon Sep 17 00:00:00 2001 From: WithoutPants <53250216+WithoutPants@users.noreply.github.com> Date: Wed, 22 May 2024 14:59:08 +1000 Subject: [PATCH] Enforce whitelist for sort values (#4865) --- graphql/schema/types/filters.graphql | 1 + pkg/sqlite/file.go | 23 ++++++++++++-- pkg/sqlite/gallery.go | 31 +++++++++++++++++-- pkg/sqlite/image.go | 30 +++++++++++++++++-- pkg/sqlite/movies.go | 29 ++++++++++++++++-- pkg/sqlite/performer.go | 36 ++++++++++++++++++++-- pkg/sqlite/scene.go | 45 ++++++++++++++++++++++++++-- pkg/sqlite/scene_marker.go | 23 ++++++++++++-- pkg/sqlite/sql.go | 26 ++++++++++++++-- pkg/sqlite/studio.go | 29 ++++++++++++++++-- pkg/sqlite/tag.go | 29 ++++++++++++++++-- 11 files changed, 275 insertions(+), 27 deletions(-) diff --git a/graphql/schema/types/filters.graphql b/graphql/schema/types/filters.graphql index 443da9b07..5d5209006 100644 --- a/graphql/schema/types/filters.graphql +++ b/graphql/schema/types/filters.graphql @@ -8,6 +8,7 @@ input FindFilterType { page: Int "use per_page = -1 to indicate all results. Defaults to 25." per_page: Int + # TODO - this should be refactored to not use a string sort: String direction: SortDirectionEnum } diff --git a/pkg/sqlite/file.go b/pkg/sqlite/file.go index 0d7f032e1..c071320c6 100644 --- a/pkg/sqlite/file.go +++ b/pkg/sqlite/file.go @@ -862,7 +862,9 @@ func (qb *FileStore) Query(ctx context.Context, options models.FileQueryOptions) return nil, err } - qb.setQuerySort(&query, findFilter) + if err := qb.setQuerySort(&query, findFilter); err != nil { + return nil, err + } query.sortAndPagination += getPagination(findFilter) result, err := qb.queryGroupedFields(ctx, options, query) @@ -911,12 +913,25 @@ func (qb *FileStore) queryGroupedFields(ctx context.Context, options models.File return ret, nil } -func (qb *FileStore) setQuerySort(query *queryBuilder, findFilter *models.FindFilterType) { +var fileSortOptions = sortOptions{ + "created_at", + "id", + "path", + "random", + "updated_at", +} + +func (qb *FileStore) setQuerySort(query *queryBuilder, findFilter *models.FindFilterType) error { if findFilter == nil || findFilter.Sort == nil || *findFilter.Sort == "" { - return + return nil } sort := findFilter.GetSort("path") + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := fileSortOptions.validateSort(sort); err != nil { + return err + } + direction := findFilter.GetDirection() switch sort { case "path": @@ -925,6 +940,8 @@ func (qb *FileStore) setQuerySort(query *queryBuilder, findFilter *models.FindFi default: query.sortAndPagination += getSort(sort, direction, "files") } + + return nil } func (qb *FileStore) captionRepository() *captionRepository { diff --git a/pkg/sqlite/gallery.go b/pkg/sqlite/gallery.go index 61ade6e6f..7ddb514d0 100644 --- a/pkg/sqlite/gallery.go +++ b/pkg/sqlite/gallery.go @@ -782,7 +782,9 @@ func (qb *GalleryStore) makeQuery(ctx context.Context, galleryFilter *models.Gal return nil, err } - qb.setGallerySort(&query, findFilter) + if err := qb.setGallerySort(&query, findFilter); err != nil { + return nil, err + } query.sortAndPagination += getPagination(findFilter) return &query, nil @@ -1100,14 +1102,35 @@ func galleryAverageResolutionCriterionHandler(qb *GalleryStore, resolution *mode } } -func (qb *GalleryStore) setGallerySort(query *queryBuilder, findFilter *models.FindFilterType) { +var gallerySortOptions = sortOptions{ + "created_at", + "date", + "file_count", + "file_mod_time", + "id", + "images_count", + "path", + "performer_count", + "random", + "rating", + "tag_count", + "title", + "updated_at", +} + +func (qb *GalleryStore) setGallerySort(query *queryBuilder, findFilter *models.FindFilterType) error { if findFilter == nil || findFilter.Sort == nil || *findFilter.Sort == "" { - return + return nil } sort := findFilter.GetSort("path") direction := findFilter.GetDirection() + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := gallerySortOptions.validateSort(sort); err != nil { + return err + } + addFileTable := func() { query.addJoins( join{ @@ -1163,6 +1186,8 @@ func (qb *GalleryStore) setGallerySort(query *queryBuilder, findFilter *models.F // Whatever the sorting, always use title/id as a final sort query.sortAndPagination += ", COALESCE(galleries.title, galleries.id) COLLATE NATURAL_CI ASC" + + return nil } func (qb *GalleryStore) GetURLs(ctx context.Context, galleryID int) ([]string, error) { diff --git a/pkg/sqlite/image.go b/pkg/sqlite/image.go index cac372991..02cd09ec7 100644 --- a/pkg/sqlite/image.go +++ b/pkg/sqlite/image.go @@ -791,7 +791,9 @@ func (qb *ImageStore) makeQuery(ctx context.Context, imageFilter *models.ImageFi return nil, err } - qb.setImageSortAndPagination(&query, findFilter) + if err := qb.setImageSortAndPagination(&query, findFilter); err != nil { + return nil, err + } return &query, nil } @@ -1051,13 +1053,35 @@ func imagePerformerTagsCriterionHandler(qb *ImageStore, tags *models.Hierarchica } } -func (qb *ImageStore) setImageSortAndPagination(q *queryBuilder, findFilter *models.FindFilterType) { +var imageSortOptions = sortOptions{ + "created_at", + "date", + "file_count", + "file_mod_time", + "filesize", + "id", + "o_counter", + "path", + "performer_count", + "random", + "rating", + "tag_count", + "title", + "updated_at", +} + +func (qb *ImageStore) setImageSortAndPagination(q *queryBuilder, findFilter *models.FindFilterType) error { sortClause := "" if findFilter != nil && findFilter.Sort != nil && *findFilter.Sort != "" { sort := findFilter.GetSort("title") direction := findFilter.GetDirection() + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := imageSortOptions.validateSort(sort); err != nil { + return err + } + // translate sort field if sort == "file_mod_time" { sort = "mod_time" @@ -1110,6 +1134,8 @@ func (qb *ImageStore) setImageSortAndPagination(q *queryBuilder, findFilter *mod } q.sortAndPagination = sortClause + getPagination(findFilter) + + return nil } func (qb *ImageStore) galleriesRepository() *joinRepository { diff --git a/pkg/sqlite/movies.go b/pkg/sqlite/movies.go index 5e5434d68..0d7c429d0 100644 --- a/pkg/sqlite/movies.go +++ b/pkg/sqlite/movies.go @@ -368,7 +368,13 @@ func (qb *MovieStore) makeQuery(ctx context.Context, movieFilter *models.MovieFi return nil, err } - query.sortAndPagination = qb.getMovieSort(findFilter) + getPagination(findFilter) + var err error + query.sortAndPagination, err = qb.getMovieSort(findFilter) + if err != nil { + return nil, err + } + + query.sortAndPagination += getPagination(findFilter) return &query, nil } @@ -466,7 +472,19 @@ func moviePerformersCriterionHandler(qb *MovieStore, performers *models.MultiCri } } -func (qb *MovieStore) getMovieSort(findFilter *models.FindFilterType) string { +var movieSortOptions = sortOptions{ + "created_at", + "date", + "duration", + "id", + "name", + "random", + "rating", + "scenes_count", + "updated_at", +} + +func (qb *MovieStore) getMovieSort(findFilter *models.FindFilterType) (string, error) { var sort string var direction string if findFilter == nil { @@ -477,6 +495,11 @@ func (qb *MovieStore) getMovieSort(findFilter *models.FindFilterType) string { direction = findFilter.GetDirection() } + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := movieSortOptions.validateSort(sort); err != nil { + return "", err + } + sortQuery := "" switch sort { case "scenes_count": // generic getSort won't work for this @@ -487,7 +510,7 @@ func (qb *MovieStore) getMovieSort(findFilter *models.FindFilterType) string { // Whatever the sorting, always use name/id as a final sort sortQuery += ", COALESCE(movies.name, movies.id) COLLATE NATURAL_CI ASC" - return sortQuery + return sortQuery, nil } func (qb *MovieStore) queryMovies(ctx context.Context, query string, args []interface{}) ([]*models.Movie, error) { diff --git a/pkg/sqlite/performer.go b/pkg/sqlite/performer.go index f10706b45..5463a6a8d 100644 --- a/pkg/sqlite/performer.go +++ b/pkg/sqlite/performer.go @@ -706,7 +706,12 @@ func (qb *PerformerStore) makeQuery(ctx context.Context, performerFilter *models return nil, err } - query.sortAndPagination = qb.getPerformerSort(findFilter) + getPagination(findFilter) + var err error + query.sortAndPagination, err = qb.getPerformerSort(findFilter) + if err != nil { + return nil, err + } + query.sortAndPagination += getPagination(findFilter) return &query, nil } @@ -1113,7 +1118,27 @@ func (qb *PerformerStore) sortByLastPlayedAt(direction string) string { return " ORDER BY (" + selectPerformerLastPlayedAtSQL + ") " + direction } -func (qb *PerformerStore) getPerformerSort(findFilter *models.FindFilterType) string { +var performerSortOptions = sortOptions{ + "birthdate", + "created_at", + "galleries_count", + "height", + "id", + "images_count", + "last_o_at", + "last_played_at", + "name", + "o_counter", + "penis_length", + "play_count", + "random", + "rating", + "scenes_count", + "tag_count", + "updated_at", +} + +func (qb *PerformerStore) getPerformerSort(findFilter *models.FindFilterType) (string, error) { var sort string var direction string if findFilter == nil { @@ -1124,6 +1149,11 @@ func (qb *PerformerStore) getPerformerSort(findFilter *models.FindFilterType) st direction = findFilter.GetDirection() } + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := performerSortOptions.validateSort(sort); err != nil { + return "", err + } + sortQuery := "" switch sort { case "tag_count": @@ -1148,7 +1178,7 @@ func (qb *PerformerStore) getPerformerSort(findFilter *models.FindFilterType) st // Whatever the sorting, always use name/id as a final sort sortQuery += ", COALESCE(performers.name, performers.id) COLLATE NATURAL_CI ASC" - return sortQuery + return sortQuery, nil } func (qb *PerformerStore) tagsRepository() *joinRepository { diff --git a/pkg/sqlite/scene.go b/pkg/sqlite/scene.go index 841dfa3f4..8c35d162c 100644 --- a/pkg/sqlite/scene.go +++ b/pkg/sqlite/scene.go @@ -1083,7 +1083,9 @@ func (qb *SceneStore) makeQuery(ctx context.Context, sceneFilter *models.SceneFi return nil, err } - qb.setSceneSort(&query, findFilter) + if err := qb.setSceneSort(&query, findFilter); err != nil { + return nil, err + } query.sortAndPagination += getPagination(findFilter) return &query, nil @@ -1522,12 +1524,47 @@ func scenePhashDistanceCriterionHandler(qb *SceneStore, phashDistance *models.Ph } } -func (qb *SceneStore) setSceneSort(query *queryBuilder, findFilter *models.FindFilterType) { +var sceneSortOptions = sortOptions{ + "bitrate", + "created_at", + "date", + "file_count", + "filesize", + "duration", + "file_mod_time", + "framerate", + "id", + "interactive", + "interactive_speed", + "last_o_at", + "last_played_at", + "movie_scene_number", + "o_counter", + "organized", + "performer_count", + "play_count", + "play_duration", + "resume_time", + "path", + "perceptual_similarity", + "random", + "rating", + "tag_count", + "title", + "updated_at", +} + +func (qb *SceneStore) setSceneSort(query *queryBuilder, findFilter *models.FindFilterType) error { if findFilter == nil || findFilter.Sort == nil || *findFilter.Sort == "" { - return + return nil } sort := findFilter.GetSort("title") + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := sceneSortOptions.validateSort(sort); err != nil { + return err + } + addFileTable := func() { query.addJoins( join{ @@ -1627,6 +1664,8 @@ func (qb *SceneStore) setSceneSort(query *queryBuilder, findFilter *models.FindF // Whatever the sorting, always use title/id as a final sort query.sortAndPagination += ", COALESCE(scenes.title, scenes.id) COLLATE NATURAL_CI ASC" + + return nil } func (qb *SceneStore) SaveActivity(ctx context.Context, id int, resumeTime *float64, playDuration *float64) (bool, error) { diff --git a/pkg/sqlite/scene_marker.go b/pkg/sqlite/scene_marker.go index e44bdf8a1..f1221cd0e 100644 --- a/pkg/sqlite/scene_marker.go +++ b/pkg/sqlite/scene_marker.go @@ -310,7 +310,9 @@ func (qb *SceneMarkerStore) makeQuery(ctx context.Context, sceneMarkerFilter *mo return nil, err } - qb.setSceneMarkerSort(&query, findFilter) + if err := qb.setSceneMarkerSort(&query, findFilter); err != nil { + return nil, err + } query.sortAndPagination += getPagination(findFilter) return &query, nil @@ -473,10 +475,26 @@ func sceneMarkerPerformersCriterionHandler(qb *SceneMarkerStore, performers *mod } } -func (qb *SceneMarkerStore) setSceneMarkerSort(query *queryBuilder, findFilter *models.FindFilterType) { +var sceneMarkerSortOptions = sortOptions{ + "created_at", + "id", + "title", + "random", + "scene_id", + "scenes_updated_at", + "seconds", + "updated_at", +} + +func (qb *SceneMarkerStore) setSceneMarkerSort(query *queryBuilder, findFilter *models.FindFilterType) error { sort := findFilter.GetSort("title") direction := findFilter.GetDirection() + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := sceneMarkerSortOptions.validateSort(sort); err != nil { + return err + } + switch sort { case "scenes_updated_at": sort = "updated_at" @@ -490,6 +508,7 @@ func (qb *SceneMarkerStore) setSceneMarkerSort(query *queryBuilder, findFilter * } query.sortAndPagination += ", scene_markers.scene_id ASC, scene_markers.seconds ASC" + return nil } func (qb *SceneMarkerStore) querySceneMarkers(ctx context.Context, query string, args []interface{}) ([]*models.SceneMarker, error) { diff --git a/pkg/sqlite/sql.go b/pkg/sqlite/sql.go index f3a3f7b2d..2c5e7d396 100644 --- a/pkg/sqlite/sql.go +++ b/pkg/sqlite/sql.go @@ -42,6 +42,30 @@ func getPaginationSQL(page int, perPage int) string { return " LIMIT " + strconv.Itoa(perPage) + " OFFSET " + strconv.Itoa(page) + " " } +const randomSeedPrefix = "random_" // prefix for random sort + +type sortOptions []string + +func (o sortOptions) validateSort(sort string) error { + if strings.HasPrefix(sort, randomSeedPrefix) { + // seed as a parameter from the UI + seedStr := sort[len(randomSeedPrefix):] + _, err := strconv.ParseUint(seedStr, 10, 64) + if err != nil { + return fmt.Errorf("invalid random seed: %s", seedStr) + } + return nil + } + + for _, v := range o { + if v == sort { + return nil + } + } + + return fmt.Errorf("invalid sort: %s", sort) +} + func getSortDirection(direction string) string { if direction != "ASC" && direction != "DESC" { return "ASC" @@ -52,8 +76,6 @@ func getSortDirection(direction string) string { func getSort(sort string, direction string, tableName string) string { direction = getSortDirection(direction) - const randomSeedPrefix = "random_" - switch { case strings.HasSuffix(sort, "_count"): var relationTableName = strings.TrimSuffix(sort, "_count") // TODO: pluralize? diff --git a/pkg/sqlite/studio.go b/pkg/sqlite/studio.go index 83425475b..e6ab03157 100644 --- a/pkg/sqlite/studio.go +++ b/pkg/sqlite/studio.go @@ -555,7 +555,12 @@ func (qb *StudioStore) makeQuery(ctx context.Context, studioFilter *models.Studi return nil, err } - query.sortAndPagination = qb.getStudioSort(findFilter) + getPagination(findFilter) + var err error + query.sortAndPagination, err = qb.getStudioSort(findFilter) + if err != nil { + return nil, err + } + query.sortAndPagination += getPagination(findFilter) return &query, nil } @@ -666,7 +671,20 @@ func studioChildCountCriterionHandler(qb *StudioStore, childCount *models.IntCri } } -func (qb *StudioStore) getStudioSort(findFilter *models.FindFilterType) string { +var studioSortOptions = sortOptions{ + "child_count", + "created_at", + "galleries_count", + "id", + "images_count", + "name", + "scenes_count", + "random", + "rating", + "updated_at", +} + +func (qb *StudioStore) getStudioSort(findFilter *models.FindFilterType) (string, error) { var sort string var direction string if findFilter == nil { @@ -677,6 +695,11 @@ func (qb *StudioStore) getStudioSort(findFilter *models.FindFilterType) string { direction = findFilter.GetDirection() } + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := studioSortOptions.validateSort(sort); err != nil { + return "", err + } + sortQuery := "" switch sort { case "scenes_count": @@ -693,7 +716,7 @@ func (qb *StudioStore) getStudioSort(findFilter *models.FindFilterType) string { // Whatever the sorting, always use name/id as a final sort sortQuery += ", COALESCE(studios.name, studios.id) COLLATE NATURAL_CI ASC" - return sortQuery + return sortQuery, nil } func (qb *StudioStore) GetImage(ctx context.Context, studioID int) ([]byte, error) { diff --git a/pkg/sqlite/tag.go b/pkg/sqlite/tag.go index bf33e04c5..cfed64bfc 100644 --- a/pkg/sqlite/tag.go +++ b/pkg/sqlite/tag.go @@ -548,7 +548,12 @@ func (qb *TagStore) Query(ctx context.Context, tagFilter *models.TagFilterType, return nil, 0, err } - query.sortAndPagination = qb.getTagSort(&query, findFilter) + getPagination(findFilter) + var err error + query.sortAndPagination, err = qb.getTagSort(&query, findFilter) + if err != nil { + return nil, 0, err + } + query.sortAndPagination += getPagination(findFilter) idsResult, countResult, err := query.executeFind(ctx) if err != nil { return nil, 0, err @@ -853,11 +858,24 @@ func tagChildCountCriterionHandler(qb *TagStore, childCount *models.IntCriterion } } +var tagSortOptions = sortOptions{ + "created_at", + "galleries_count", + "id", + "images_count", + "name", + "performers_count", + "random", + "scene_markers_count", + "scenes_count", + "updated_at", +} + func (qb *TagStore) getDefaultTagSort() string { return getSort("name", "ASC", "tags") } -func (qb *TagStore) getTagSort(query *queryBuilder, findFilter *models.FindFilterType) string { +func (qb *TagStore) getTagSort(query *queryBuilder, findFilter *models.FindFilterType) (string, error) { var sort string var direction string if findFilter == nil { @@ -868,6 +886,11 @@ func (qb *TagStore) getTagSort(query *queryBuilder, findFilter *models.FindFilte direction = findFilter.GetDirection() } + // CVE-2024-32231 - ensure sort is in the list of allowed sorts + if err := tagSortOptions.validateSort(sort); err != nil { + return "", err + } + sortQuery := "" switch sort { case "scenes_count": @@ -886,7 +909,7 @@ func (qb *TagStore) getTagSort(query *queryBuilder, findFilter *models.FindFilte // Whatever the sorting, always use name/id as a final sort sortQuery += ", COALESCE(tags.name, tags.id) COLLATE NATURAL_CI ASC" - return sortQuery + return sortQuery, nil } func (qb *TagStore) queryTags(ctx context.Context, query string, args []interface{}) ([]*models.Tag, error) {