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.
This commit is contained in:
parent
3c3d4b863d
commit
6df0d6c8f5
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue