From 7adc4717ec8e4f8fe678010866e936cf024f498d Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:34:32 -0800 Subject: Include file extension checks in attachment API (#32151) From testing, I found that issue posters and users with repository write access are able to edit attachment names in a way that circumvents the instance-level file extension restrictions using the edit attachment APIs. This snapshot adds checks for these endpoints. --- routers/api/v1/repo/issue_attachment.go | 13 ++++++++++--- routers/api/v1/repo/issue_comment_attachment.go | 13 ++++++++++--- routers/api/v1/repo/release_attachment.go | 13 ++++++++++--- 3 files changed, 30 insertions(+), 9 deletions(-) (limited to 'routers') diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 27c7af2282..d0bcadde37 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -12,7 +12,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -181,7 +181,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -247,6 +247,8 @@ func EditIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "404": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/validationError" // "423": // "$ref": "#/responses/repoArchivedError" @@ -261,8 +263,13 @@ func EditIssueAttachment(ctx *context.APIContext) { attachment.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attachment); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Attachment.AllowedTypes, attachment); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attachment)) diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 0863ebd182..a556a803e5 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -14,7 +14,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -189,7 +189,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -263,6 +263,8 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "404": // "$ref": "#/responses/error" + // "422": + // "$ref": "#/responses/validationError" // "423": // "$ref": "#/responses/repoArchivedError" attach := getIssueCommentAttachmentSafeWrite(ctx) @@ -275,8 +277,13 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attach); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Attachment.AllowedTypes, attach); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach)) } diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 4a2371e012..ed6cc8e1ea 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/convert" @@ -234,7 +234,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Create a new attachment and save the file - attach, err := attachment.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{ + attach, err := attachment_service.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -291,6 +291,8 @@ func EditReleaseAttachment(ctx *context.APIContext) { // responses: // "201": // "$ref": "#/responses/Attachment" + // "422": + // "$ref": "#/responses/validationError" // "404": // "$ref": "#/responses/notFound" @@ -322,8 +324,13 @@ func EditReleaseAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attach); err != nil { + if err := attachment_service.UpdateAttachment(ctx, setting.Repository.Release.AllowedTypes, attach); err != nil { + if upload.IsErrFileTypeForbidden(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + return } ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach)) } -- cgit v1.2.3