mirror of
https://github.com/filebrowser/filebrowser.git
synced 2026-07-17 16:36:49 +00:00
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.
This commit is contained in:
parent
883a36f02f
commit
ec13054671
4 changed files with 91 additions and 7 deletions
|
|
@ -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))"
|
||||
>
|
||||
<i class="material-icons">content_paste_go</i>
|
||||
|
|
|
|||
2
frontend/src/types/api.d.ts
vendored
2
frontend/src/types/api.d.ts
vendored
|
|
@ -25,7 +25,7 @@ interface Share {
|
|||
path: string;
|
||||
expire?: any;
|
||||
userID?: number;
|
||||
token?: string;
|
||||
hasPassword?: boolean;
|
||||
username?: string;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue