From 31f7971e2d0aa1202d9bd00318344c994fb5d647 Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 14:55:50 +0100 Subject: [PATCH 01/16] fix(server): prevent non-admin MaxUserSize quota bypass The UpdateUser handler checked MaxTTL and MaxFileSize to prevent non-admin users from modifying their own quotas, but did not check MaxUserSize. A non-admin user could increase their own storage quota by sending a modified MaxUserSize value in the update request. Add MaxUserSize to the existing quota check and add a dedicated test. --- server/handlers/user.go | 2 +- server/handlers/user_test.go | 59 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/server/handlers/user.go b/server/handlers/user.go index 4b535722..54c1d9f3 100644 --- a/server/handlers/user.go +++ b/server/handlers/user.go @@ -108,7 +108,7 @@ func UpdateUser(ctx *context.Context, resp http.ResponseWriter, req *http.Reques ctx.Forbidden("can't grant yourself admin right, nice try!") return } - if userParams.MaxTTL != user.MaxTTL || userParams.MaxFileSize != user.MaxFileSize { + if userParams.MaxTTL != user.MaxTTL || userParams.MaxFileSize != user.MaxFileSize || userParams.MaxUserSize != user.MaxUserSize { ctx.Forbidden("can't edit your own quota, nice try!") return } diff --git a/server/handlers/user_test.go b/server/handlers/user_test.go index 170d76f6..cab2c71c 100644 --- a/server/handlers/user_test.go +++ b/server/handlers/user_test.go @@ -368,6 +368,65 @@ func TestUpdateUser_FailGrant(t *testing.T) { require.False(t, updatedUser.IsAdmin) } +func TestUpdateUser_FailQuotaMaxUserSize(t *testing.T) { + ctx := newTestingContext(common.NewConfiguration()) + + originalUser := &common.User{ + ID: "local:user", + Provider: "local", + Login: "user", + Password: "password", + Email: "user@root.gg", + Name: "user", + MaxFileSize: 1234, + MaxUserSize: 5678, + MaxTTL: 1234, + IsAdmin: false, + } + ctx.SetUser(originalUser) + + err := ctx.GetMetadataBackend().CreateUser(originalUser) + require.NoError(t, err) + + var updateParams = &struct { + ID string `json:"id,omitempty"` + Provider string `json:"provider"` + Login string `json:"login,omitempty"` + Password string `json:"password"` + Name string `json:"name,omitempty"` + Email string `json:"email,omitempty"` + IsAdmin bool `json:"admin"` + MaxFileSize int64 `json:"maxFileSize"` + MaxUserSize int64 `json:"maxUserSize"` + MaxTTL int `json:"maxTTL"` + }{ + ID: "local:user", + Provider: "local", + Login: "user", + Password: "password", + Email: "user@root.gg", + Name: "user", + MaxFileSize: 1234, + MaxUserSize: -1, // Trying to set unlimited + MaxTTL: 1234, + } + + userJSON, err := utils.ToJsonString(updateParams) + require.NoError(t, err) + + req, err := http.NewRequest("GET", "/", bytes.NewBufferString(userJSON)) + require.NoError(t, err, "unable to create new request") + + rr := ctx.NewRecorder(req) + UpdateUser(ctx, rr, req) + context.TestForbidden(t, rr, "can't edit your own quota") + + // Verify the user was not modified + updatedUser, err := ctx.GetMetadataBackend().GetUser(originalUser.ID) + require.NoError(t, err) + require.Equal(t, int64(5678), updatedUser.MaxUserSize, "MaxUserSize should not have changed") +} + func TestUpdateUser_OKGrant(t *testing.T) { ctx := newTestingContext(common.NewConfiguration()) From 4b03a1d4da769d698541e61693b8a04501a58a75 Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 15:12:55 +0100 Subject: [PATCH 02/16] fix(server): return 404 for cross-upload file access When requesting a file that exists but belongs to a different upload, the server returned HTTP 500 instead of 404. This allowed an attacker to distinguish between non-existent file IDs (404) and file IDs that exist in other uploads (500), enabling file ID enumeration. Return 404 with the same message as a genuinely missing file. --- server/middleware/file.go | 2 +- server/middleware/file_test.go | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/server/middleware/file.go b/server/middleware/file.go index d22c4ca2..596cdfdd 100644 --- a/server/middleware/file.go +++ b/server/middleware/file.go @@ -45,7 +45,7 @@ func File(ctx *context.Context, next http.Handler) http.Handler { } if file.UploadID != upload.ID { - ctx.InternalServerError("invalid file upload id", nil) + ctx.NotFound("file %s not found", fileID) return } diff --git a/server/middleware/file_test.go b/server/middleware/file_test.go index f68d4c83..9a0a8ae9 100644 --- a/server/middleware/file_test.go +++ b/server/middleware/file_test.go @@ -104,6 +104,42 @@ func TestFileInvalidFileName(t *testing.T) { context.TestInvalidParameter(t, rr, "file name") } +func TestFileCrossUpload(t *testing.T) { + ctx := newTestingContext(common.NewConfiguration()) + + // Create upload A with a file + uploadA := &common.Upload{} + fileA := uploadA.NewFile() + fileA.Name = "filename" + uploadA.InitializeForTests() + err := ctx.GetMetadataBackend().CreateUpload(uploadA) + require.NoError(t, err, "create upload A error") + + // Create upload B (no file) + uploadB := &common.Upload{} + uploadB.InitializeForTests() + err = ctx.GetMetadataBackend().CreateUpload(uploadB) + require.NoError(t, err, "create upload B error") + + // Set context to upload B, but request file from upload A + ctx.SetUpload(uploadB) + + req, err := http.NewRequest("GET", "", &bytes.Buffer{}) + require.NoError(t, err, "unable to create new request") + + vars := map[string]string{ + "fileID": fileA.ID, + "filename": fileA.Name, + } + req = mux.SetURLVars(req, vars) + + rr := ctx.NewRecorder(req) + File(ctx, common.DummyHandler).ServeHTTP(rr, req) + + // Should return 404 Not Found (not 500) to prevent file ID enumeration + context.TestNotFound(t, rr, "file "+fileA.ID+" not found") +} + func TestFile(t *testing.T) { ctx := newTestingContext(common.NewConfiguration()) From 876850d7a9d280683709352a1e24b6e9a69b4c99 Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 15:15:36 +0100 Subject: [PATCH 03/16] fix(server): reject empty password when protection is requested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a client sent protectedByPassword=true with an empty password, the server silently created an unprotected upload. This gave users a false sense of security — they believed their upload was password- protected when it was not. Now return a 400 error with message 'upload password is empty' when the client explicitly requests password protection but provides no password. --- server/context/upload.go | 3 +++ server/context/upload_test.go | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/server/context/upload.go b/server/context/upload.go index ff3662a3..d4f0ea3a 100644 --- a/server/context/upload.go +++ b/server/context/upload.go @@ -38,6 +38,9 @@ func (ctx *Context) CreateUpload(params *common.Upload) (upload *common.Upload, } // Handle Basic Auth parameters + if params.ProtectedByPassword && params.Password == "" { + return nil, fmt.Errorf("upload password is empty") + } err = ctx.setBasicAuth(upload, params.Login, params.Password) if err != nil { return nil, err diff --git a/server/context/upload_test.go b/server/context/upload_test.go index 1b7a45d6..7fc05b63 100644 --- a/server/context/upload_test.go +++ b/server/context/upload_test.go @@ -258,6 +258,22 @@ func TestUpload_PasswordDefaultLogin(t *testing.T) { require.True(t, upload.ProtectedByPassword) } +func TestUpload_PasswordEmptyWithProtectedFlag(t *testing.T) { + ctx := newTestContext() + ctx.config.FeaturePassword = common.FeatureEnabled + + // Client sends protectedByPassword=true but no password => error + upload, err := ctx.CreateUpload(&common.Upload{ProtectedByPassword: true, Password: ""}) + common.RequireError(t, err, "upload password is empty") + require.Nil(t, upload) + + // Client sends protectedByPassword=true with valid password => ok + upload, err = ctx.CreateUpload(&common.Upload{ProtectedByPassword: true, Password: "password"}) + require.NoError(t, err) + require.NotNil(t, upload) + require.True(t, upload.ProtectedByPassword) +} + func TestUpload_CommentsDisabled(t *testing.T) { ctx := newTestContext() ctx.config.FeatureComments = common.FeatureDisabled @@ -364,7 +380,7 @@ func TestCreateUpload(t *testing.T) { params.ID = "id" params.UploadToken = "token" params.IsAdmin = true - params.ProtectedByPassword = true + params.RemoteIP = "1.3.3.7" params.TTL = 42 params.ExtendTTL = true From 347de0b4a292b067ddd5140e0f30ea0d9f88c45c Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 15:20:40 +0100 Subject: [PATCH 04/16] fix(server): handle logout gracefully when auth is disabled The Logout handler called GetAuthenticator() which panics when the authenticator is nil (authentication disabled). The panic recovery middleware then returned HTTP 500. Add GetAuthenticatorSafe() that returns nil instead of panicking, and use it in the Logout handler to return 200 gracefully. --- server/context/context.go | 8 ++++++++ server/handlers/misc.go | 8 ++++++-- server/handlers/misc_test.go | 13 +++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/server/context/context.go b/server/context/context.go index 282969c3..ad7b478d 100644 --- a/server/context/context.go +++ b/server/context/context.go @@ -155,6 +155,14 @@ func (ctx *Context) SetAuthenticator(authenticator *common.SessionAuthenticator) ctx.authenticator = authenticator } +// GetAuthenticatorSafe get authenticator from the context (nil-safe, no panic). +func (ctx *Context) GetAuthenticatorSafe() *common.SessionAuthenticator { + ctx.mu.RLock() + defer ctx.mu.RUnlock() + + return ctx.authenticator +} + // GetMetrics get metrics from the context. func (ctx *Context) GetMetrics() *common.PlikMetrics { ctx.mu.RLock() diff --git a/server/handlers/misc.go b/server/handlers/misc.go index d43bcd0f..30486fe8 100644 --- a/server/handlers/misc.go +++ b/server/handlers/misc.go @@ -30,9 +30,13 @@ func GetConfiguration(ctx *context.Context, resp http.ResponseWriter, req *http. common.WriteJSONResponse(resp, ctx.GetConfig()) } -// Logout return the server configuration +// Logout delete session cookies func Logout(ctx *context.Context, resp http.ResponseWriter, req *http.Request) { - common.Logout(resp, ctx.GetAuthenticator()) + authenticator := ctx.GetAuthenticatorSafe() + if authenticator == nil { + return + } + common.Logout(resp, authenticator) } // GetQrCode return a QRCode for the requested URL diff --git a/server/handlers/misc_test.go b/server/handlers/misc_test.go index 0489a66d..9e6981ad 100644 --- a/server/handlers/misc_test.go +++ b/server/handlers/misc_test.go @@ -182,6 +182,19 @@ func TestLogout(t *testing.T) { context.TestOK(t, rr) } +func TestLogoutNoAuthenticator(t *testing.T) { + ctx := newTestingContext(common.NewConfiguration()) + // Clear the authenticator to simulate auth being disabled + ctx.SetAuthenticator(nil) + + req, err := http.NewRequest("GET", "/logout", bytes.NewBuffer([]byte{})) + require.NoError(t, err, "unable to create new request") + + rr := ctx.NewRecorder(req) + Logout(ctx, rr, req) + context.TestOK(t, rr) +} + func TestGetRedirectionURL(t *testing.T) { ctx := newTestingContext(common.NewConfiguration()) From 37ea79705f76bd750bee14d4d6223736122d843f Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 15:33:47 +0100 Subject: [PATCH 05/16] fix(server): neutralize SVG, XML and JavaScript content types Extend content-type neutralization to cover SVG, XML, and JavaScript in addition to the existing HTML, Flash, and PDF handling. All dangerous types are now consistently forced to application/octet-stream to prevent XSS via SVG onload handlers, XML rendering, or script execution. Document the neutralization rules in the security guide. --- docs/guide/security.md | 17 +++++++++++++++++ server/handlers/get_file.go | 17 +++++++++-------- server/handlers/get_file_test.go | 28 +++++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/docs/guide/security.md b/docs/guide/security.md index e8f82c59..4640c65b 100644 --- a/docs/guide/security.md +++ b/docs/guide/security.md @@ -6,6 +6,23 @@ Plik allows users to upload and serve any content as-is. Hosting untrusted conte For security reasons, Plik doesn't trust user-provided MIME types and relies solely on server-side detection. This means some files may not render properly in browsers or embedded viewers that require the correct MIME type. +### Dangerous Content-Type Neutralization + +Plik automatically neutralizes content types that could execute code in the browser: + +| Original type | Served as | Reason | +|---|---|---| +| `text/html`, `*html*` | `application/octet-stream` | Prevents inline script execution | +| `image/svg+xml`, `*svg*` | `application/octet-stream` | SVG can contain `onload` JavaScript handlers | +| `text/xml`, `*xml*` | `application/octet-stream` | XML can be parsed and rendered by browsers | +| `application/javascript`, `*javascript*` | `application/octet-stream` | Prevents script execution | +| `application/x-shockwave-flash` | `application/octet-stream` | Flash content | +| `application/pdf` | `application/octet-stream` | PDF can contain JavaScript | + +::: tip +This protection is always active regardless of `EnhancedWebSecurity`. Use the `?dl=true` query parameter to force a download with `Content-Disposition: attachment`. +::: + ::: warning Office format detection limitation Office formats like `.pptx`, `.docx`, and `.xlsx` are ZIP archives internally, so Go's built-in MIME detector (`http.DetectContentType`) identifies them as `application/zip` instead of their proper types (e.g., `application/vnd.openxmlformats-officedocument.presentationml.presentation`). ::: diff --git a/server/handlers/get_file.go b/server/handlers/get_file.go index e8df0e7f..337ad404 100644 --- a/server/handlers/get_file.go +++ b/server/handlers/get_file.go @@ -67,14 +67,15 @@ func GetFile(ctx *context.Context, resp http.ResponseWriter, req *http.Request) } } - // Avoid rendering potentially dangerous content types in the browser - // HTML could execute inline scripts, SVG/XML can contain onload handlers (SECU-10) - if strings.Contains(file.Type, "html") || strings.Contains(file.Type, "svg") || file.Type == "text/xml" || file.Type == "text/xml; charset=utf-8" { - file.Type = "application/octet-stream" - } - - // Force the download of the following types as they are blocked by the CSP Header and won't display properly. - if file.Type == "" || strings.Contains(file.Type, "flash") || strings.Contains(file.Type, "pdf") { + // Neutralize content types that could execute code in the browser + // Force download as binary to prevent XSS via inline scripts, SVG onload handlers, etc. + if file.Type == "" || + strings.Contains(file.Type, "html") || + strings.Contains(file.Type, "svg") || + strings.Contains(file.Type, "xml") || + strings.Contains(file.Type, "javascript") || + strings.Contains(file.Type, "flash") || + strings.Contains(file.Type, "pdf") { file.Type = "application/octet-stream" } diff --git a/server/handlers/get_file_test.go b/server/handlers/get_file_test.go index 740abe02..b4e65b70 100644 --- a/server/handlers/get_file_test.go +++ b/server/handlers/get_file_test.go @@ -257,7 +257,7 @@ func TestGetHtmlFile(t *testing.T) { GetFile(ctx, rr, req) context.TestOK(t, rr) - require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type"), "invalid content type") + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type"), "HTML files should be neutralized to octet-stream") } func TestGetSvgFile(t *testing.T) { @@ -312,6 +312,32 @@ func TestGetXmlFile(t *testing.T) { require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type"), "XML files should be neutralized to octet-stream") } +func TestGetJavascriptFile(t *testing.T) { + config := common.NewConfiguration() + ctx := newTestingContext(config) + + upload := &common.Upload{} + upload.InitializeForTests() + + file := upload.NewFile() + file.Type = "application/javascript" + file.Status = "uploaded" + err := createTestFile(ctx, file, bytes.NewBuffer([]byte("data"))) + require.NoError(t, err, "unable to create test file") + + ctx.SetUpload(upload) + ctx.SetFile(file) + + req, err := http.NewRequest("GET", "/file/", bytes.NewBuffer([]byte{})) + require.NoError(t, err, "unable to create new request") + + rr := ctx.NewRecorder(req) + GetFile(ctx, rr, req) + context.TestOK(t, rr) + + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type"), "JS files should be neutralized to octet-stream") +} + func TestGetFileNoType(t *testing.T) { config := common.NewConfiguration() ctx := newTestingContext(config) From 920706081146890a3190ddc43a2ff3c2766f37c1 Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 15:33:57 +0100 Subject: [PATCH 06/16] chore: gitignore security review documents --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 840c76a8..afa101cc 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,4 @@ plikd_*.cfg .vscode plik.iml coverage.out +/plans \ No newline at end of file From 063192d56b361fe349f27f75c7dae8564049c7ea Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 16:05:30 +0100 Subject: [PATCH 07/16] fix(server): limit upload comment length to 32KB The comments field had no server-side length validation, allowing clients to submit arbitrarily large comments. Add a 10,000 character maximum and return an error when exceeded. --- server/context/upload.go | 4 ++++ server/context/upload_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/server/context/upload.go b/server/context/upload.go index d4f0ea3a..33b92f54 100644 --- a/server/context/upload.go +++ b/server/context/upload.go @@ -181,6 +181,10 @@ func (ctx *Context) setParams(upload *common.Upload, params *common.Upload) (err upload.Comments = params.Comments } + if len(upload.Comments) > 32768 { + return fmt.Errorf("comment is too long (max 32768 bytes)") + } + upload.E2EE = params.E2EE if upload.E2EE != "" && !common.IsValidE2EEScheme(upload.E2EE) { return fmt.Errorf("invalid e2ee scheme %q", upload.E2EE) diff --git a/server/context/upload_test.go b/server/context/upload_test.go index 7fc05b63..150c7f84 100644 --- a/server/context/upload_test.go +++ b/server/context/upload_test.go @@ -2,6 +2,7 @@ package context import ( "net" + "strings" "testing" "time" @@ -326,6 +327,24 @@ func TestUpload_CommentsForced(t *testing.T) { require.Equal(t, "comments", upload.Comments) } +func TestUpload_CommentsMaxLength(t *testing.T) { + ctx := newTestContext() + ctx.config.FeatureComments = common.FeatureEnabled + + // 32768 bytes should be accepted + longComment := strings.Repeat("a", 32768) + upload, err := ctx.CreateUpload(&common.Upload{Comments: longComment}) + require.NoError(t, err) + require.NotNil(t, upload) + require.Len(t, upload.Comments, 32768) + + // 32769 bytes should be rejected + tooLongComment := strings.Repeat("a", 32769) + upload, err = ctx.CreateUpload(&common.Upload{Comments: tooLongComment}) + common.RequireError(t, err, "comment is too long") + require.Nil(t, upload) +} + func TestUpload_ExtendTTLDisabled(t *testing.T) { ctx := newTestContext() ctx.config.FeatureExtendTTL = common.FeatureDisabled From 586dd354ddbe85a6f4330a60cc11802789f4a8bd Mon Sep 17 00:00:00 2001 From: "ca.mathieu" Date: Sun, 1 Mar 2026 16:07:09 +0100 Subject: [PATCH 08/16] fix(webapp): add rel="noopener noreferrer" to download links Anchor tags opening in a new tab without rel="noopener" allow the opened page to access window.opener. Add the attribute to both the filename link and the download button in FileRow.vue. --- webapp/src/components/FileRow.vue | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/FileRow.vue b/webapp/src/components/FileRow.vue index 062349b6..55a265c6 100644 --- a/webapp/src/components/FileRow.vue +++ b/webapp/src/components/FileRow.vue @@ -129,7 +129,8 @@ function fileUrl() { + target="_blank" + rel="noopener noreferrer"> {{ file.fileName }}