diff --git a/changelog/unreleased/validate-space-names.md b/changelog/unreleased/validate-space-names.md new file mode 100644 index 000000000..d96e255dd --- /dev/null +++ b/changelog/unreleased/validate-space-names.md @@ -0,0 +1,9 @@ +Enhancement: Validate space names + +We now return `BAD REQUEST` when space names are +- too long (max 255 characters) +- containing evil characters (`/`, `\`, `.`, `\\`, `:`, `?`, `*`, `"`, `>`, `<`, `|`) + +Additionally leading and trailing spaces will be removed silently. + +https://github.com/owncloud/ocis/pull/4955 diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 05dd0aeb3..e75b09a86 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -33,6 +33,20 @@ import ( merrors "go-micro.dev/v4/errors" ) +var ( + _invalidSpaceNameCharacters = []string{`/`, `\`, `.`, `:`, `?`, `*`, `"`, `>`, `<`, `|`} + _maxSpaceNameLength = 255 + + // ErrNameTooLong is thrown when the spacename is too long + ErrNameTooLong = fmt.Errorf("spacename must be smaller than %d", _maxSpaceNameLength) + + // ErrNameEmpty is thrown when the spacename is empty + ErrNameEmpty = errors.New("spacename must not be empty") + + // ErrForbiddenCharacter is thrown when the spacename contains an invalid character + ErrForbiddenCharacter = fmt.Errorf("spacenames must not contain %v", _invalidSpaceNameCharacters) +) + // GetDrives lists all drives the current user has access to func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { g.getDrives(w, r, false) @@ -229,10 +243,10 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid body schema definition") return } - spaceName := *drive.Name - if spaceName == "" { - logger.Debug().Str("name", spaceName).Msg("could not create drive: invalid name") - errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid name") + spaceName := strings.TrimSpace(*drive.Name) + if err := validateSpaceName(spaceName); err != nil { + logger.Debug().Str("name", spaceName).Err(err).Msg("could not create drive: name validation failed") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("invalid spacename: %s", err.Error())) return } @@ -376,7 +390,14 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) { } if drive.Name != nil { - updateSpaceRequest.StorageSpace.Name = *drive.Name + spacename := strings.TrimSpace(*drive.Name) + if err := validateSpaceName(spacename); err != nil { + logger.Info().Err(err).Msg("could not update drive: spacename invalid") + errorcode.GeneralException.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + + updateSpaceRequest.StorageSpace.Name = spacename } if drive.Quota.HasTotal() { @@ -901,3 +922,21 @@ func sortSpaces(req *godata.GoDataRequest, spaces []*libregraph.Drive) ([]*libre sort.Sort(sorter) return spaces, nil } + +func validateSpaceName(name string) error { + if name == "" { + return ErrNameEmpty + } + + if len(name) > _maxSpaceNameLength { + return ErrNameTooLong + } + + for _, c := range _invalidSpaceNameCharacters { + if strings.Contains(name, c) { + return ErrForbiddenCharacter + } + } + + return nil +} diff --git a/services/graph/pkg/service/v0/drives_test.go b/services/graph/pkg/service/v0/drives_test.go index 954b24c4c..ac18991b4 100644 --- a/services/graph/pkg/service/v0/drives_test.go +++ b/services/graph/pkg/service/v0/drives_test.go @@ -7,6 +7,7 @@ import ( "github.com/CiscoM31/godata" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/stretchr/testify/assert" + "github.com/test-go/testify/require" ) type sortTest struct { @@ -121,3 +122,35 @@ func TestSort(t *testing.T) { assert.Equal(t, test.DrivesSorted, sorted) } } + +func TestSpaceNameValidation(t *testing.T) { + // set max length + _maxSpaceNameLength = 10 + + testCases := []struct { + Alias string + SpaceName string + ExpectedError error + }{ + {"Happy Path", "Space", nil}, + {"Just not too Long", "abcdefghij", nil}, + {"Too Long", "abcdefghijk", ErrNameTooLong}, + {"Empty", "", ErrNameEmpty}, + {"Contains /", "Space/", ErrForbiddenCharacter}, + {`Contains \`, `Space\`, ErrForbiddenCharacter}, + {`Contains \\`, `Space\\`, ErrForbiddenCharacter}, + {"Contains .", "Space.", ErrForbiddenCharacter}, + {"Contains :", "Space:", ErrForbiddenCharacter}, + {"Contains ?", "Sp?ace", ErrForbiddenCharacter}, + {"Contains *", "Spa*ce", ErrForbiddenCharacter}, + {`Contains "`, `"Space"`, ErrForbiddenCharacter}, + {`Contains >`, `Sp>ce`, ErrForbiddenCharacter}, + {`Contains <`, `S