From 730099d2aaf12905d9a77a4389f2a5920a943a19 Mon Sep 17 00:00:00 2001 From: sysops Date: Fri, 12 Jun 2026 23:26:31 +0200 Subject: [PATCH] =?UTF-8?q?fix(sec):=20Cross-Tenant-IDOR=20bei=20IMAP-Kont?= =?UTF-8?q?en=20schlie=C3=9Fen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit domain_admin sah und konnte IMAP-Konten (inkl. Credentials) fremder Tenants auflisten, löschen, synchronisieren und umkonfigurieren, da Store.List() für Admins ungefiltert alle Konten lieferte und die Einzelhandler nur den Owner, nicht den Tenant prüften. - Store.List() filtert jetzt nach tenant_id, außer für superadmin - Store.Create() setzt tenant_id beim Anlegen - Alle Einzelhandler (delete/start-import/progress/sync/update) prüfen zusätzlich tenantAccessAllowed() --- internal/api/import_handlers.go | 34 ++++++++++++++++++++++++++++++++- internal/imap/store.go | 19 +++++++++++------- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/internal/api/import_handlers.go b/internal/api/import_handlers.go index 0544d9c..e6f2604 100644 --- a/internal/api/import_handlers.go +++ b/internal/api/import_handlers.go @@ -15,6 +15,17 @@ import ( // ── IMAP handlers ───────────────────────────────────────────────────────── +// tenantAccessAllowed checks whether the given session may access an IMAP/POP3 +// account belonging to accTenantID. Superadmins (sess.TenantID == nil) may +// access any tenant. Other admins may only access accounts within their own +// tenant (accTenantID must be set and match). +func tenantAccessAllowed(sess *auth.Session, accTenantID *int64) bool { + if sess.TenantID == nil { + return true + } + return accTenantID != nil && *accTenantID == *sess.TenantID +} + func (s *Server) handleListImap(w http.ResponseWriter, r *http.Request) { if s.imapStore == nil { writeError(w, http.StatusServiceUnavailable, "IMAP not configured") @@ -23,7 +34,7 @@ func (s *Server) handleListImap(w http.ResponseWriter, r *http.Request) { sess := sessionFromCtx(r.Context()) // SEC-03: Use HasRole to correctly check admin privileges (domain_admin, admin, superadmin). isAdmin := auth.HasRole(sess.Role, userstore.RoleDomainAdmin) - accounts, err := s.imapStore.List(r.Context(), sess.Username, isAdmin) + accounts, err := s.imapStore.List(r.Context(), sess.Username, isAdmin, sess.TenantID) if err != nil { writeError(w, http.StatusInternalServerError, "failed to list IMAP accounts") return @@ -75,6 +86,7 @@ func (s *Server) handleCreateImap(w http.ResponseWriter, r *http.Request) { TLS: req.TLS, Username: req.Username, ExcludedFolders: req.ExcludedFolders, + TenantID: sess.TenantID, } created, err := s.imapStore.Create(r.Context(), acc, req.Password) @@ -109,6 +121,10 @@ func (s *Server) handleDeleteImap(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusForbidden, "access denied") return } + if !tenantAccessAllowed(sess, acc.TenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } if err := s.imapStore.Delete(r.Context(), id); err != nil { writeError(w, http.StatusInternalServerError, "failed to delete account") @@ -197,6 +213,10 @@ func (s *Server) handleStartImport(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusForbidden, "access denied") return } + if !tenantAccessAllowed(sess, acc.TenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } if acc.Status == "running" { writeError(w, http.StatusConflict, "import already running") @@ -233,6 +253,10 @@ func (s *Server) handleImapProgress(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusForbidden, "access denied") return } + if !tenantAccessAllowed(sess, acc.TenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } writeJSON(w, http.StatusOK, acc) } @@ -261,6 +285,10 @@ func (s *Server) handleSyncNow(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusForbidden, "access denied") return } + if !tenantAccessAllowed(sess, acc.TenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } if err := s.imapScheduler.TriggerSync(r.Context(), id); err != nil { s.logger.Error("trigger sync failed", "err", err) @@ -299,6 +327,10 @@ func (s *Server) handleUpdateImapInterval(w http.ResponseWriter, r *http.Request writeError(w, http.StatusForbidden, "access denied") return } + if !tenantAccessAllowed(sess, acc.TenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } var req struct { SyncIntervalMin int `json:"sync_interval_min"` diff --git a/internal/imap/store.go b/internal/imap/store.go index c539b18..e0bda4f 100644 --- a/internal/imap/store.go +++ b/internal/imap/store.go @@ -139,10 +139,10 @@ func (s *Store) Create(ctx context.Context, acc Account, password string) (*Acco } row := s.pool.QueryRow(ctx, ` - INSERT INTO imap_accounts (owner, name, host, port, tls, username, password_enc, excluded_folders) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + INSERT INTO imap_accounts (owner, name, host, port, tls, username, password_enc, excluded_folders, tenant_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING id, created_at`, - acc.Owner, acc.Name, acc.Host, acc.Port, acc.TLS, acc.Username, enc, acc.ExcludedFolders, + acc.Owner, acc.Name, acc.Host, acc.Port, acc.TLS, acc.Username, enc, acc.ExcludedFolders, acc.TenantID, ) if err := row.Scan(&acc.ID, &acc.CreatedAt); err != nil { @@ -180,16 +180,21 @@ func scanRow(row scanner) (Account, error) { return a, err } -// List returns IMAP accounts. Admins see all accounts; regular users see only their own. -func (s *Store) List(ctx context.Context, owner string, isAdmin bool) ([]Account, error) { +// List returns IMAP accounts. Superadmins (tenantID == nil) see all accounts; +// other admins (tenantID != nil) see all accounts within their own tenant; +// regular users see only their own accounts. +func (s *Store) List(ctx context.Context, owner string, isAdmin bool, tenantID *int64) ([]Account, error) { var rows pgx.Rows var err error q := `SELECT` + selectColumns + `FROM imap_accounts` - if isAdmin { + switch { + case isAdmin && tenantID == nil: rows, err = s.pool.Query(ctx, q+` ORDER BY id`) - } else { + case isAdmin: + rows, err = s.pool.Query(ctx, q+` WHERE tenant_id = $1 ORDER BY id`, *tenantID) + default: rows, err = s.pool.Query(ctx, q+` WHERE owner = $1 ORDER BY id`, owner) } if err != nil {