From ec130546713c44cd24556907552ac554c7f809c9 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Sat, 27 Jun 2026 09:08:56 +0200 Subject: [PATCH] fix(share): stop exposing password hash and bypass token in share API (GHSA-833g-cqhp-h72j) The share management endpoints serialized the storage struct directly, returning the bcrypt password_hash (crackable offline) and the bypass token for every share an authenticated caller could list, with admins seeing them for all users. Return a response DTO that exposes only whether a share is password-protected (hasPassword) and drops both secrets. The storage struct keeps its tags so the secrets stay persisted and the server-side auth/public flows are unchanged. Updates the frontend to use hasPassword and adds a regression test. --- frontend/src/components/prompts/Share.vue | 2 +- frontend/src/types/api.d.ts | 2 +- http/share.go | 40 ++++++++++++++--- http/share_test.go | 54 +++++++++++++++++++++++ 4 files changed, 91 insertions(+), 7 deletions(-) diff --git a/frontend/src/components/prompts/Share.vue b/frontend/src/components/prompts/Share.vue index 282902fb..c51904d1 100644 --- a/frontend/src/components/prompts/Share.vue +++ b/frontend/src/components/prompts/Share.vue @@ -38,7 +38,7 @@ class="action" :aria-label="$t('buttons.copyDownloadLinkToClipboard')" :title="$t('buttons.copyDownloadLinkToClipboard')" - :disabled="!!link.password_hash" + :disabled="!!link.hasPassword" @click="copyToClipboard(buildDownloadLink(link))" > content_paste_go diff --git a/frontend/src/types/api.d.ts b/frontend/src/types/api.d.ts index c1592a21..f4864724 100644 --- a/frontend/src/types/api.d.ts +++ b/frontend/src/types/api.d.ts @@ -25,7 +25,7 @@ interface Share { path: string; expire?: any; userID?: number; - token?: string; + hasPassword?: boolean; username?: string; } diff --git a/http/share.go b/http/share.go index 3d05fe05..8ed4feb4 100644 --- a/http/share.go +++ b/http/share.go @@ -19,6 +19,36 @@ import ( "golang.org/x/crypto/bcrypt" ) +// shareResponse is the client-facing representation of a share. It deliberately +// omits the server-side secrets of share.Link — the bcrypt PasswordHash (which +// would be crackable offline) and the bypass Token — exposing only whether the +// share is password-protected via HasPassword. +type shareResponse struct { + Hash string `json:"hash"` + Path string `json:"path"` + UserID uint `json:"userID"` + Expire int64 `json:"expire"` + HasPassword bool `json:"hasPassword"` +} + +func toShareResponse(l *share.Link) *shareResponse { + return &shareResponse{ + Hash: l.Hash, + Path: l.Path, + UserID: l.UserID, + Expire: l.Expire, + HasPassword: l.PasswordHash != "", + } +} + +func toShareResponses(links []*share.Link) []*shareResponse { + res := make([]*shareResponse, 0, len(links)) + for _, l := range links { + res = append(res, toShareResponse(l)) + } + return res +} + func withPermShare(fn handleFunc) handleFunc { return withUser(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { if !d.user.Perm.Share || !d.user.Perm.Download { @@ -40,7 +70,7 @@ var shareListHandler = withPermShare(func(w http.ResponseWriter, r *http.Request s, err = d.store.Share.FindByUserID(d.user.ID) } if errors.Is(err, fberrors.ErrNotExist) { - return renderJSON(w, r, []*share.Link{}) + return renderJSON(w, r, []*shareResponse{}) } if err != nil { @@ -54,7 +84,7 @@ var shareListHandler = withPermShare(func(w http.ResponseWriter, r *http.Request return s[i].Expire < s[j].Expire }) - return renderJSON(w, r, s) + return renderJSON(w, r, toShareResponses(s)) }) var shareGetsHandler = withPermShare(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { @@ -68,14 +98,14 @@ var shareGetsHandler = withPermShare(func(w http.ResponseWriter, r *http.Request s, err = d.store.Share.Gets(r.URL.Path, d.user.ID) } if errors.Is(err, fberrors.ErrNotExist) { - return renderJSON(w, r, []*share.Link{}) + return renderJSON(w, r, []*shareResponse{}) } if err != nil { return http.StatusInternalServerError, err } - return renderJSON(w, r, s) + return renderJSON(w, r, toShareResponses(s)) }) func getSharesForAdminPath(d *data, path string) ([]*share.Link, error) { @@ -204,7 +234,7 @@ var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request return http.StatusInternalServerError, err } - return renderJSON(w, r, s) + return renderJSON(w, r, toShareResponse(s)) }) func getSharePasswordHash(body share.CreateBody) (data []byte, statuscode int, err error) { diff --git a/http/share_test.go b/http/share_test.go index 8029acc5..c03a9393 100644 --- a/http/share_test.go +++ b/http/share_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -92,6 +93,59 @@ func TestAdminShareGetsHandlerMatchesOwnerScope(t *testing.T) { } } +// Regression for the share secret exposure (GHSA-833g-cqhp-h72j): the share API +// must not serialize the bcrypt password hash or the bypass token, while still +// persisting them server-side so password-protected shares keep working. +func TestSharePostHandlerDoesNotLeakSecrets(t *testing.T) { + root := t.TempDir() + userScope := filepath.Join(root, "user") + if err := os.MkdirAll(userScope, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(userScope, "file.txt"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + + key := []byte("test-signing-key") + perm := users.Permissions{Share: true, Download: true} + st := scopedUserStorage(t, userScope, perm, key) + signed := signToken(t, perm, key) + + body := `{"password":"ShareSecret123!","expires":"24","unit":"hours"}` + req, _ := http.NewRequest(http.MethodPost, "/file.txt", strings.NewReader(body)) + req.Header.Set("X-Auth", signed) + rec := httptest.NewRecorder() + handle(sharePostHandler, "", st, &settings.Server{Root: root}).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%q", rec.Code, rec.Body.String()) + } + + var resp map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if _, ok := resp["password_hash"]; ok { + t.Errorf("VULNERABLE: response leaks password_hash: %s", rec.Body.String()) + } + if _, ok := resp["token"]; ok { + t.Errorf("VULNERABLE: response leaks token: %s", rec.Body.String()) + } + if resp["hasPassword"] != true { + t.Errorf("expected hasPassword=true, got %v", resp["hasPassword"]) + } + + // The secrets must still be persisted server-side (storm uses the JSON codec, + // so the storage struct's tags must keep emitting them). + stored, err := st.Share.GetByHash(resp["hash"].(string)) + if err != nil { + t.Fatalf("share not stored: %v", err) + } + if stored.PasswordHash == "" || stored.Token == "" { + t.Fatalf("server-side secrets not persisted: hash=%q token=%q", stored.PasswordHash, stored.Token) + } +} + func signShareTestToken(t *testing.T, id uint, username string, perm users.Permissions, key []byte) string { t.Helper()