web/forgejo
8
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-02-22 11:25:47 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(sec): web route delete runner

Browse source 
The web route to delete action runners did not check if the ID that was
given belonged to the context it was requested in, this made it possible
to delete every existing runner of a instance by a authenticated user.

The code was reworked to ensure that the caller of the delete
runner function retrieved the runner by ID and then checks if it belongs
to the context it was requested in, although this is not an optimal
solution it is consistent with the context checking of other code for
runners.
This commit is contained in:
Gusted 2025-01-25 08:51:59 +01:00 • committed by Earl Warren
parent 94845020e8
commit f359ebeea5
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
4 changed files with 20 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
models/actions/runner.go
View file

@ -282,27 +282,22 @@ func UpdateRunner(ctx context.Context, r *ActionRunner, cols ...string) error {
}
// DeleteRunner deletes a runner by given ID.
func DeleteRunner(ctx context.Context, id int64) error {
runner, err := GetRunnerByID(ctx, id)
if err != nil {
return err
}
func DeleteRunner(ctx context.Context, r *ActionRunner) error {
// Replace the UUID, which was either based on the secret's first 16 bytes or an UUIDv4,
// with a sequence of 8 0xff bytes followed by the little-endian version of the record's
// identifier. This will prevent the deleted record's identifier from colliding with any
// new record.
b := make([]byte, 8)
binary.LittleEndian.PutUint64(b, uint64(id))
runner.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
binary.LittleEndian.PutUint64(b, uint64(r.ID))
r.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
b[0], b[1], b[2], b[3], b[4], b[5], b[6], b[7])
err = UpdateRunner(ctx, runner, "UUID")
err := UpdateRunner(ctx, r, "UUID")
if err != nil {
return err
}
_, err = db.DeleteByID[ActionRunner](ctx, id)
_, err = db.DeleteByID[ActionRunner](ctx, r.ID)
return err
}

2
models/actions/runner_test.go
View file

@ -34,7 +34,7 @@ func TestDeleteRunner(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
err := DeleteRunner(db.DefaultContext, recordID)
err := DeleteRunner(db.DefaultContext, &ActionRunner{ID: recordID})
require.NoError(t, err)
var after ActionRunner

2
routers/web/repo/setting/runners.go
View file

@ -179,7 +179,7 @@ func RunnerDeletePost(ctx *context.Context) {
ctx.ServerError("getRunnersCtx", err)
return
}
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.OwnerID, rCtx.RepoID, rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
}
func RedirectToDefaultSetting(ctx *context.Context) {

15
routers/web/shared/actions/runners.go
View file

@ -142,10 +142,21 @@ func RunnerResetRegistrationToken(ctx *context.Context, ownerID, repoID int64, r
}
// RunnerDeletePost response for deleting a runner
func RunnerDeletePost(ctx *context.Context, runnerID int64,
func RunnerDeletePost(ctx *context.Context, runnerID, ownerID, repoID int64,
successRedirectTo, failedRedirectTo string,
) {
if err := actions_model.DeleteRunner(ctx, runnerID); err != nil {
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
if err != nil {
ctx.ServerError("GetRunnerByID", err)
return
}
if !runner.Editable(ownerID, repoID) {
ctx.NotFound("Editable", util.NewPermissionDeniedErrorf("no permission to edit this runner"))
return
}
if err := actions_model.DeleteRunner(ctx, runner); err != nil {
log.Warn("DeleteRunnerPost.UpdateRunner failed: %v, url: %s", err, ctx.Req.URL)
ctx.Flash.Warning(ctx.Tr("actions.runners.delete_runner_failed"))