Merge pull request #11231 from 2403905/issue-7180

[full-ci] limited the length of tags

changelog/unreleased/add-tags-validation.md

@@ -0,0 +1,5 @@
+Enhancement: Limit length of tags
+
+We limited the length of tags to avoid DOS attacks against the ocis server.
+
+https://github.com/owncloud/ocis/pull/11231

go.mod

@@ -67,7 +67,7 @@ require (
 	github.com/open-policy-agent/opa v0.70.0
 	github.com/orcaman/concurrent-map v1.0.0
 	github.com/owncloud/libre-graph-api-go v1.0.5-0.20250217093259-fa3804be6c27
-	github.com/owncloud/reva/v2 v2.0.0-20250331084351-00f64db78848
+	github.com/owncloud/reva/v2 v2.0.0-20250415081347-32419403823e
 	github.com/pkg/errors v0.9.1
 	github.com/pkg/xattr v0.4.10
 	github.com/prometheus/client_golang v1.20.5

go.sum

@@ -881,8 +881,8 @@ github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CF
 github.com/ovh/go-ovh v1.1.0/go.mod h1:AxitLZ5HBRPyUd+Zl60Ajaag+rNTdVXWIkzfrVuTXWA=
 github.com/owncloud/libre-graph-api-go v1.0.5-0.20250217093259-fa3804be6c27 h1:ID8s5lGBntmrlI6TbDAjTzRyHucn3bVM2wlW+HBplv4=
 github.com/owncloud/libre-graph-api-go v1.0.5-0.20250217093259-fa3804be6c27/go.mod h1:+gT+x62AS9u2Farh9wE2uYmgdvTg0MQgsSI62D+xoRg=
-github.com/owncloud/reva/v2 v2.0.0-20250331084351-00f64db78848 h1:eWOevrc619bmhJwV9tzT3Ak1oFU9Nx9gufLFiCETN9Q=
-github.com/owncloud/reva/v2 v2.0.0-20250331084351-00f64db78848/go.mod h1:1QUFTq8Q2tjzwY3g+Y1dHIO4tPYqTovV6ScacW3sTPs=
+github.com/owncloud/reva/v2 v2.0.0-20250415081347-32419403823e h1:ukea680IP8n6y8uhuICLbb3hjc8SFp8kDlCUZmsSoAU=
+github.com/owncloud/reva/v2 v2.0.0-20250415081347-32419403823e/go.mod h1:1QUFTq8Q2tjzwY3g+Y1dHIO4tPYqTovV6ScacW3sTPs=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
 github.com/pablodz/inotifywaitgo v0.0.7 h1:1ii49dGBnRn0t1Sz7RGZS6/NberPEDQprwKHN49Bv6U=

services/frontend/pkg/config/config.go

@@ -57,6 +57,7 @@ type Config struct {
 	ServiceAccount   ServiceAccount        `yaml:"service_account"`
 
 	PasswordPolicy PasswordPolicy `yaml:"password_policy"`
+	Validation     Validation     `yaml:"validation"`
 
 	ConfigurableNotifications bool `yaml:"configurable_notifications" env:"FRONTEND_CONFIGURABLE_NOTIFICATIONS" desc:"Allow configuring notifications via web client." introductionVersion:"7.1"`
 
@@ -193,3 +194,7 @@ type PasswordPolicy struct {
 	MinSpecialCharacters   int    `yaml:"min_special_characters" env:"OCIS_PASSWORD_POLICY_MIN_SPECIAL_CHARACTERS;FRONTEND_PASSWORD_POLICY_MIN_SPECIAL_CHARACTERS" desc:"Define the minimum number of characters from the special characters list to be present. Defaults to 1 if not set." introductionVersion:"5.0"`
 	BannedPasswordsList    string `yaml:"banned_passwords_list" env:"OCIS_PASSWORD_POLICY_BANNED_PASSWORDS_LIST;FRONTEND_PASSWORD_POLICY_BANNED_PASSWORDS_LIST" desc:"Path to the 'banned passwords list' file. This only impacts public link password validation. See the documentation for more details." introductionVersion:"5.0"`
 }
+
+type Validation struct {
+	MaxTagLength int `yaml:"max_tag_length" env:"OCIS_MAX_TAG_LENGTH" desc:"Define the maximum tag length. Defaults to 100 if not set. Set to 0 to not limit the tag length. Changes only impact the validation of new tags." introductionVersion:"%%NEXT%%"`
+}

services/frontend/pkg/config/defaults/defaultconfig.go

@@ -138,6 +138,9 @@ func DefaultConfig() *config.Config {
 			MinDigits:              1,
 			MinSpecialCharacters:   1,
 		},
+		Validation: config.Validation{
+			MaxTagLength: 100,
+		},
 	}
 }
 

services/frontend/pkg/revaconfig/config.go

@@ -225,6 +225,9 @@ func FrontendConfigFromStruct(cfg *config.Config, logger log.Logger) (map[string
 									"delete_disabled":               !cfg.LDAPServerWriteEnabled,
 									"change_password_self_disabled": changePasswordDisabled,
 								},
+								"tags": map[string]interface{}{
+									"max_tag_length": cfg.Validation.MaxTagLength,
+								},
 							},
 							"checksums": map[string]interface{}{
 								"supported_types":       cfg.Checksums.SupportedTypes,

services/graph/pkg/config/config.go

@@ -36,6 +36,8 @@ type Config struct {
 	Keycloak       Keycloak       `yaml:"keycloak"`
 	ServiceAccount ServiceAccount `yaml:"service_account"`
 
+	Validation Validation `yaml:"validation"`
+
 	Context context.Context `yaml:"-"`
 }
 
@@ -153,3 +155,7 @@ type ServiceAccount struct {
 	ServiceAccountID     string `yaml:"service_account_id" env:"OCIS_SERVICE_ACCOUNT_ID;GRAPH_SERVICE_ACCOUNT_ID" desc:"The ID of the service account the service should use. See the 'auth-service' service description for more details." introductionVersion:"5.0"`
 	ServiceAccountSecret string `yaml:"service_account_secret" env:"OCIS_SERVICE_ACCOUNT_SECRET;GRAPH_SERVICE_ACCOUNT_SECRET" desc:"The service account secret." introductionVersion:"5.0"`
 }
+
+type Validation struct {
+	MaxTagLength int `yaml:"max_tag_length" env:"OCIS_MAX_TAG_LENGTH" desc:"Define the maximum tag length. Defaults to 100 if not set. Set to 0 to not limit the tag length. Changes only impact the validation of new tags." introductionVersion:"%%NEXT%%"`
+}

services/graph/pkg/config/defaults/defaultconfig.go

@@ -127,6 +127,9 @@ func DefaultConfig() *config.Config {
 		UnifiedRoles: config.UnifiedRoles{
 			AvailableRoles: nil, // will be populated with defaults in EnsureDefaults
 		},
+		Validation: config.Validation{
+			MaxTagLength: 100,
+		},
 	}
 }
 

services/graph/pkg/service/v0/tags.go

@@ -109,7 +109,12 @@ func (g Graph) AssignTags(w http.ResponseWriter, r *http.Request) {
 	}
 
 	allTags := tags.New(currentTags)
-	if !allTags.Add(assignment.Tags...) {
+	ok, err := allTags.AddValidated(tags.MaxLengthValidator(g.config.Validation.MaxTagLength), assignment.Tags...)
+	if err != nil {
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
+		return
+	}
+	if !ok {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "no new tags in createtagsrequest or maximum reached")
 		return
 	}

vendor/github.com/owncloud/reva/v2/pkg/owncloud/ocs/capabilities.go

@@ -114,6 +114,7 @@ type CapabilitiesCore struct {
 type CapabilitiesGraph struct {
 	PersonalDataExport ocsBool                `json:"personal-data-export" xml:"personal-data-export" mapstructure:"personal_data_export"`
 	Users              CapabilitiesGraphUsers `json:"users" xml:"users" mapstructure:"users"`
+	Tags               CapabilitiesGraphTags  `json:"tags" xml:"tags" mapstructure:"tags"`
 }
 
 // CapabilitiesPasswordPolicy hold the password policy capabilities
@@ -135,6 +136,11 @@ type CapabilitiesGraphUsers struct {
 	ChangePasswordSelfDisabled ocsBool  `json:"change_password_self_disabled" xml:"change_password_self_disabled" mapstructure:"change_password_self_disabled"`
 }
 
+// CapabilitiesGraphTags holds the graph tags capabilities
+type CapabilitiesGraphTags struct {
+	MaxTagLength int `json:"max_tag_length" xml:"max_tag_length" mapstructure:"max_tag_length"`
+}
+
 // Status holds basic status information
 type Status struct {
 	Installed      ocsBool `json:"installed" xml:"installed"`

vendor/github.com/owncloud/reva/v2/pkg/tags/tags.go

@@ -19,7 +19,9 @@
 package tags
 
 import (
+	"fmt"
 	"strings"
+	"unicode/utf8"
 )
 
 var (
@@ -42,35 +44,45 @@ type Tags struct {
 // New creates a Tag struct from a slice of tags, e.g. ["tag1", "tag2"] or a list of tags, e.g. "tag1,tag2"
 func New(ts ...string) *Tags {
 	t := &Tags{sep: _tagsep, maxtags: _maxtags, exists: make(map[string]bool), t: make([]string, 0)}
-	t.addTags(ts)
+	t.addTags(t.normalize(ts))
 	return t
 }
 
 // Add appends a list of new tags and returns true if at least one was appended
 func (t *Tags) Add(ts ...string) bool {
-	return len(t.addTags(ts)) > 0
+	return len(t.addTags(t.normalize(ts))) > 0
+}
+
+// AddValidated appends a list of new tags and validates them using the provided validator function
+// It returns true if at least one was appended and an error if the validation failed
+func (t *Tags) AddValidated(validator func([]string) error, ts ...string) (bool, error) {
+	newTags := t.normalize(ts)
+	err := validator(newTags)
+	if err != nil {
+		return false, err
+	}
+	return len(t.addTags(newTags)) > 0, nil
 }
 
 // Remove removes a list of tags and returns true if at least one was removed
 func (t *Tags) Remove(s ...string) bool {
 	var removed bool
+	tags := t.normalize(s)
 
-	for _, tt := range s {
-		for _, tag := range strings.Split(tt, t.sep) {
-			if !t.exists[tag] {
-				continue
-			}
-
-			for i, tt := range t.t {
-				if tt == tag {
-					t.t = append(t.t[:i], t.t[i+1:]...)
-					break
-				}
-			}
-
-			delete(t.exists, tag)
-			removed = true
+	for _, tag := range tags {
+		if !t.exists[tag] {
+			continue
 		}
+
+		for i, tt := range t.t {
+			if tt == tag {
+				t.t = append(t.t[:i], t.t[i+1:]...)
+				break
+			}
+		}
+
+		delete(t.exists, tag)
+		removed = true
 	}
 	return removed
 }
@@ -86,31 +98,70 @@ func (t *Tags) AsSlice() []string {
 }
 
 // adds the tags and returns a list of added tags
+// the function receiving a normalized slice of tags, e.g.["tag1", "tag2"]
 func (t *Tags) addTags(s []string) []string {
-	added := make([]string, 0)
-	for _, tt := range s {
-		for _, tag := range strings.Split(tt, t.sep) {
-			if tag == "" {
-				// ignore empty tags
-				continue
-			}
-
-			if t.exists[tag] {
-				// tag is already existing
-				continue
-			}
-
-			if t.numtags >= t.maxtags {
-				// max number of tags reached. We return silently without warning anyone
-				break
-			}
-
-			added = append(added, tag)
-			t.exists[tag] = true
-			t.numtags++
+	added := make([]string, 0, len(t.t)+len(s))
+	for _, tag := range s {
+		if tag == "" {
+			// ignore empty tags
+			continue
 		}
+
+		if t.exists[tag] {
+			// tag is already existing
+			continue
+		}
+
+		if t.numtags >= t.maxtags {
+			// max number of tags reached. We return silently without warning anyone
+			break
+		}
+
+		added = append(added, tag)
+		t.exists[tag] = true
+		t.numtags++
 	}
 
 	t.t = append(added, t.t...)
 	return added
 }
+
+// normalize splits the tags and removes empty tags
+// the function receiving a slice of tags, e.g.["tag1", "tag2"] or a list of tags, e.g."tag1,tag2" or mixed.
+func (t *Tags) normalize(s []string) []string {
+	res := make([]string, 0, t.maxtags/2)
+	for _, tt := range s {
+		for _, tag := range strings.Split(tt, t.sep) {
+			ttr := strings.TrimSpace(tag)
+			if ttr == "" {
+				// ignore empty tags
+				continue
+			}
+			res = append(res, ttr)
+		}
+	}
+	return res
+}
+
+// MaxLengthValidator returns a function that validates the length of each tag in a slice
+func MaxLengthValidator(maxTagLength int) func([]string) error {
+	if maxTagLength <= 0 {
+		return func(tags []string) error { return nil }
+	}
+	return func(tags []string) error {
+		t := make([]string, 0, maxTagLength)
+		for _, tag := range tags {
+			if !utf8.ValidString(tag) {
+				return fmt.Errorf("tag [%s] contains invalid characters", tag)
+			}
+			if utf8.RuneCount([]byte(tag)) > maxTagLength {
+				t = append(t, tag)
+				continue
+			}
+		}
+		if len(t) > 0 {
+			return fmt.Errorf("tag [%s] too long, max length is %d", strings.Join(t, ", "), maxTagLength)
+		}
+		return nil
+	}
+}

vendor/modules.txt

@@ -1219,7 +1219,7 @@ github.com/orcaman/concurrent-map
 # github.com/owncloud/libre-graph-api-go v1.0.5-0.20250217093259-fa3804be6c27
 ## explicit; go 1.18
 github.com/owncloud/libre-graph-api-go
-# github.com/owncloud/reva/v2 v2.0.0-20250331084351-00f64db78848
+# github.com/owncloud/reva/v2 v2.0.0-20250415081347-32419403823e
 ## explicit; go 1.22.7
 github.com/owncloud/reva/v2/cmd/revad/internal/grace
 github.com/owncloud/reva/v2/cmd/revad/runtime