From 6df0d6c8f54b01db6464c42810e40e55f12b481b Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 9 Apr 2026 17:20:26 +0200 Subject: [PATCH] feat(tasks): cap repeat_after at 10 years to harden repeating-task handler Add MaxTaskRepeatAfterSeconds (10 years in seconds) and reject any create/update that tries to store a value outside [0, cap] with a new ErrInvalidTaskRepeatInterval (error code 4029). Defense-in-depth alongside the arithmetic fix in addRepeatIntervalToTime: keeps stored values well away from int64 overflow and bounds the range of inputs a future refactor could trip over. --- pkg/models/error.go | 28 +++++++++++++++++++ pkg/models/tasks.go | 20 ++++++++++++++ pkg/models/tasks_test.go | 59 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) diff --git a/pkg/models/error.go b/pkg/models/error.go index 756f2ca93..92c11fd5f 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -553,6 +553,34 @@ func (err ErrTaskCannotBeEmpty) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeTaskCannotBeEmpty, Message: "You must provide at least a task title."} } +// ErrInvalidTaskRepeatInterval represents an error where the provided +// task repeat interval is outside the allowed range. +type ErrInvalidTaskRepeatInterval struct { + RepeatAfter int64 +} + +// IsErrInvalidTaskRepeatInterval checks if an error is ErrInvalidTaskRepeatInterval. +func IsErrInvalidTaskRepeatInterval(err error) bool { + _, ok := err.(ErrInvalidTaskRepeatInterval) + return ok +} + +func (err ErrInvalidTaskRepeatInterval) Error() string { + return fmt.Sprintf("Invalid task repeat interval. [RepeatAfter: %d]", err.RepeatAfter) +} + +// ErrCodeInvalidTaskRepeatInterval holds the unique world-error code of this error. +const ErrCodeInvalidTaskRepeatInterval = 4029 + +// HTTPError holds the http error description. +func (err ErrInvalidTaskRepeatInterval) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusBadRequest, + Code: ErrCodeInvalidTaskRepeatInterval, + Message: fmt.Sprintf("The task repeat interval must be between 0 and %d seconds (10 years).", MaxTaskRepeatAfterSeconds), + } +} + // ErrTaskDoesNotExist represents a "ErrProjectDoesNotExist" kind of error. Used if the project does not exist. type ErrTaskDoesNotExist struct { ID int64 diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index c719b4267..d484bb314 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -47,6 +47,18 @@ const ( TaskRepeatModeFromCurrentDate ) +// MaxTaskRepeatAfterSeconds caps repeat_after at ten years. Sized to +// stay far from int64 overflow when multiplied out in nanoseconds, and +// ten years is already well past any legitimate recurrence. +const MaxTaskRepeatAfterSeconds int64 = 10 * 365 * 24 * 3600 + +func validateRepeatAfter(repeatAfter int64) error { + if repeatAfter < 0 || repeatAfter > MaxTaskRepeatAfterSeconds { + return ErrInvalidTaskRepeatInterval{RepeatAfter: repeatAfter} + } + return nil +} + // Task represents a task in a project type Task struct { // The unique, numeric id of this task. @@ -873,6 +885,10 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool, setB return ErrTaskCannotBeEmpty{} } + if err := validateRepeatAfter(t.RepeatAfter); err != nil { + return err + } + // Check if the project exists p, err := GetProjectSimpleByID(s, t.ProjectID) if err != nil { @@ -1165,6 +1181,10 @@ func (t *Task) updateSingleTask(s *xorm.Session, a web.Auth, fields []string) (e } } + if err := validateRepeatAfter(t.RepeatAfter); err != nil { + return err + } + // If the task is being moved between projects, make sure to move the bucket + index as well if t.ProjectID != 0 && ot.ProjectID != t.ProjectID { t.Index, err = calculateNextTaskIndex(s, t.ProjectID) diff --git a/pkg/models/tasks_test.go b/pkg/models/tasks_test.go index c794899ee..536e3a173 100644 --- a/pkg/models/tasks_test.go +++ b/pkg/models/tasks_test.go @@ -988,6 +988,65 @@ func TestUpdateDone(t *testing.T) { }) } +func TestTask_RepeatAfterCap(t *testing.T) { + const maxRepeat int64 = 10 * 365 * 24 * 3600 + + t.Run("create rejects repeat_after above cap", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + usr := &user.User{ID: 1, Username: "user1"} + task := &Task{ + Title: "nope", + ProjectID: 1, + RepeatAfter: maxRepeat + 1, + } + err := task.Create(s, usr) + require.Error(t, err) + assert.True(t, IsErrInvalidTaskRepeatInterval(err)) + }) + + t.Run("create accepts repeat_after at cap", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + usr := &user.User{ID: 1, Username: "user1"} + task := &Task{ + Title: "ok", + ProjectID: 1, + RepeatAfter: maxRepeat, + } + require.NoError(t, task.Create(s, usr)) + require.NoError(t, s.Commit()) + }) + + t.Run("update rejects repeat_after above cap", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + usr := &user.User{ID: 1, Username: "user1"} + task := &Task{ + ID: 1, + RepeatAfter: maxRepeat + 1, + } + err := task.Update(s, usr) + require.Error(t, err) + assert.True(t, IsErrInvalidTaskRepeatInterval(err)) + }) +} + +func TestErrInvalidTaskRepeatInterval(t *testing.T) { + err := ErrInvalidTaskRepeatInterval{RepeatAfter: 999999999999} + assert.True(t, IsErrInvalidTaskRepeatInterval(err)) + assert.False(t, IsErrInvalidTaskRepeatInterval(ErrTaskCannotBeEmpty{})) + httpErr := err.HTTPError() + assert.Equal(t, 400, httpErr.HTTPCode) + assert.Equal(t, ErrCodeInvalidTaskRepeatInterval, httpErr.Code) +} + func TestUpdateDone_DoSRegression_AncientDueDate(t *testing.T) { // GHSA-r4fg-73rc-hhh7: ancient due_date + 1s interval used to spin // for billions of iterations. The <1s assertion catches a regression