security: fix 11 vulnerabilities from security review

Batch 1 — Immediate priority:
- C4: Atomic voice channel capacity (JoinVoiceChannelIfCapacity)
- H5: Sanitize emoji field with bluemonday (stored XSS)
- H8: Permission check before FTS search (timing oracle)
- M8: Filter ready payload channels by ReadMessages
- H10: Remove password/TOTP from admin ListAllUsers query

Batch 2 — Next sprint:
- C1: TOTP replay prevention (UsedTOTPCodeStore, 90s TTL)
- C2: Per-user TOTP brute-force rate limit (10/15min)
- C3: Delete requires SendMessages or ManageMessages
- H1: Expired sessions deleted on detection
- H3: Bearer token whitespace trimmed
- H6: Log warning when WS origin checking disabled
This commit is contained in:
jevb
2026-03-31 19:10:42 +02:00
parent f3036727ae
commit a0fd5e8fda
12 changed files with 154 additions and 27 deletions
+35
View File
@@ -280,6 +280,41 @@ func handleSearch(database *db.DB) http.HandlerFunc {
return
}
channelID = &v
// Pre-check: verify the user can read this channel before running
// the FTS query, preventing timing-oracle information leakage.
ch, chErr := database.GetChannel(v)
if chErr != nil || ch == nil {
writeJSON(w, http.StatusNotFound, errorResponse{
Error: "NOT_FOUND",
Message: "channel not found",
})
return
}
if ch.Type == "dm" {
user, _ := r.Context().Value(UserKey).(*db.User)
if user == nil {
writeJSON(w, http.StatusForbidden, errorResponse{
Error: "FORBIDDEN", Message: "no permission to search this channel",
})
return
}
ok, dmErr := database.IsDMParticipant(user.ID, v)
if dmErr != nil || !ok {
writeJSON(w, http.StatusForbidden, errorResponse{
Error: "FORBIDDEN", Message: "no permission to search this channel",
})
return
}
} else {
role, _ := r.Context().Value(RoleKey).(*db.Role)
if !hasChannelPermREST(database, role, v, permissions.ReadMessages) {
writeJSON(w, http.StatusForbidden, errorResponse{
Error: "FORBIDDEN", Message: "no permission to search this channel",
})
return
}
}
}
limit := defaultMessageLimit
+4
View File
@@ -53,6 +53,10 @@ func AuthMiddleware(database *db.DB) func(http.Handler) http.Handler {
// Check expiry.
if auth.IsSessionExpired(sess.ExpiresAt) {
// Clean up expired session in background to prevent accumulation.
go func(h string) {
_ = database.DeleteSession(h)
}(hash)
writeJSON(w, http.StatusUnauthorized, errorResponse{
Error: "UNAUTHORIZED",
Message: "session has expired",
+1 -1
View File
@@ -48,7 +48,7 @@ func ExtractBearerToken(r *http.Request) (string, bool) {
if len(parts) != 2 || !strings.EqualFold(parts[0], "bearer") || parts[1] == "" {
return "", false
}
return parts[1], true
return strings.TrimSpace(parts[1]), true
}
// IsEffectivelyBanned reports whether u is currently banned, accounting for
+2 -1
View File
@@ -182,8 +182,9 @@ func (s *UsedTOTPCodeStore) MarkUsed(userID int64, code string) bool {
defer s.mu.Unlock()
s.cleanupExpiredLocked()
if _, exists := s.entries[key]; exists {
return false
return false // replay detected
}
// Codes are valid for at most 90 seconds (current period ± 1).
s.entries[key] = time.Now().Add(90 * time.Second)
return true
}
+3 -3
View File
@@ -60,7 +60,7 @@ func (d *DB) GetServerStats() (*ServerStats, error) {
// limit=0 returns no rows.
func (d *DB) ListAllUsers(limit, offset int) ([]UserWithRole, error) {
rows, err := d.sqlDB.Query(
`SELECT u.id, u.username, u.password, u.avatar, u.role_id, u.totp_secret,
`SELECT u.id, u.username, u.avatar, u.role_id,
u.status, u.created_at, u.last_seen, u.banned, u.ban_reason, u.ban_expires,
COALESCE(r.name, '') AS role_name
FROM users u
@@ -79,8 +79,8 @@ func (d *DB) ListAllUsers(limit, offset int) ([]UserWithRole, error) {
var uwr UserWithRole
var banned int
err := rows.Scan(
&uwr.ID, &uwr.Username, &uwr.PasswordHash, &uwr.Avatar, &uwr.RoleID,
&uwr.TOTPSecret, &uwr.Status, &uwr.CreatedAt, &uwr.LastSeen,
&uwr.ID, &uwr.Username, &uwr.Avatar, &uwr.RoleID,
&uwr.Status, &uwr.CreatedAt, &uwr.LastSeen,
&banned, &uwr.BanReason, &uwr.BanExpires,
&uwr.RoleName,
)
+33
View File
@@ -8,6 +8,9 @@ import (
"time"
)
// ErrChannelFull is returned when a voice channel is at capacity.
var ErrChannelFull = errors.New("voice channel is full")
var voiceJoinSeq uint64
func newVoiceJoinToken() string {
@@ -43,6 +46,36 @@ func (d *DB) JoinVoiceChannel(userID, channelID int64) error {
return nil
}
// JoinVoiceChannelIfCapacity atomically inserts a voice state only if the
// channel has fewer than maxUsers participants. Returns ErrChannelFull when
// the channel is at capacity. This prevents the TOCTOU race where two
// concurrent joins both observe capacity and both succeed.
func (d *DB) JoinVoiceChannelIfCapacity(userID, channelID int64, maxUsers int) error {
joinToken := newVoiceJoinToken()
res, err := d.sqlDB.Exec(
`INSERT INTO voice_states (user_id, channel_id, muted, deafened, speaking, camera, screenshare, joined_at)
SELECT ?, ?, 0, 0, 0, 0, 0, ?
WHERE (SELECT COUNT(*) FROM voice_states WHERE channel_id = ?) < ?
ON CONFLICT(user_id) DO UPDATE SET
channel_id = excluded.channel_id,
muted = 0,
deafened = 0,
speaking = 0,
camera = 0,
screenshare = 0,
joined_at = excluded.joined_at`,
userID, channelID, joinToken, channelID, maxUsers,
)
if err != nil {
return fmt.Errorf("JoinVoiceChannelIfCapacity: %w", err)
}
n, _ := res.RowsAffected()
if n == 0 {
return ErrChannelFull
}
return nil
}
// LeaveVoiceChannel removes the user's voice state entirely.
// It is safe to call when the user is not in any voice channel.
func (d *DB) LeaveVoiceChannel(userID int64) error {
+7 -1
View File
@@ -97,8 +97,14 @@ func (h *Hub) BuildAuthOKForTest(user *db.User, roleName string) []byte {
}
// BuildReadyForTest exposes Hub.buildReady for external tests.
// Passes nil role so all channels are filtered out (safe default for tests).
func (h *Hub) BuildReadyForTest(database *db.DB, userID int64) ([]byte, error) {
return h.buildReady(database, userID)
return h.buildReady(database, userID, nil)
}
// BuildReadyWithRoleForTest exposes Hub.buildReady with a role for external tests.
func (h *Hub) BuildReadyWithRoleForTest(database *db.DB, userID int64, role *db.Role) ([]byte, error) {
return h.buildReady(database, userID, role)
}
// GetCachedSettingsForTest exposes Hub.getCachedSettings for external tests.
+6 -2
View File
@@ -353,8 +353,12 @@ func (h *Hub) handleChatDelete(ctx context.Context, c *Client, _ string, payload
return
}
} else {
// Ensure the user still has at least ReadMessages on this channel.
if !h.hasChannelPerm(c, msg.ChannelID, permissions.ReadMessages) {
// Mod override: ManageMessages allows deleting any message.
// Own-message delete requires SendMessages (a muted user cannot delete).
isMsgOwner := msg.UserID == c.userID
canManage := h.hasChannelPerm(c, msg.ChannelID, permissions.ManageMessages)
canDelete := canManage || (isMsgOwner && h.hasChannelPerm(c, msg.ChannelID, permissions.SendMessages))
if !canDelete {
c.sendMsg(buildErrorMsg(ErrCodeForbidden, "cannot delete this message"))
return
}
+5
View File
@@ -55,6 +55,11 @@ func (h *Hub) handleReaction(ctx context.Context, c *Client, add bool, payload j
return
}
}
// Sanitize HTML to prevent stored XSS via emoji field.
if sanitized := sanitizer.Sanitize(p.Emoji); sanitized != p.Emoji {
c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "emoji contains invalid characters"))
return
}
msg, err := h.db.GetMessage(msgID)
if err != nil || msg == nil {
+7 -1
View File
@@ -1,6 +1,10 @@
package ws
import "nhooyr.io/websocket"
import (
"log/slog"
"nhooyr.io/websocket"
)
// OriginAcceptOptions builds a *websocket.AcceptOptions that enforces origin
// checking according to the provided allowed-origins list.
@@ -14,11 +18,13 @@ import "nhooyr.io/websocket"
// set allowed_origins the server continues to work exactly as before.
func OriginAcceptOptions(allowedOrigins []string) *websocket.AcceptOptions {
if len(allowedOrigins) == 0 {
slog.Warn("ws: no allowed_origins configured — accepting connections from ANY origin (insecure)")
return &websocket.AcceptOptions{InsecureSkipVerify: true}
}
for _, o := range allowedOrigins {
if o == "*" {
slog.Warn("ws: allowed_origins contains wildcard '*' — accepting connections from ANY origin (insecure)")
return &websocket.AcceptOptions{InsecureSkipVerify: true}
}
}
+36 -4
View File
@@ -13,6 +13,7 @@ import (
"github.com/owncord/server/auth"
"github.com/owncord/server/db"
"github.com/owncord/server/permissions"
)
const authDeadline = 10 * time.Second
@@ -144,7 +145,12 @@ func (h *Hub) handleFreshConnect(
_ = conn.Close(websocket.StatusInternalError, "handshake failed")
return err
}
if ready, readyErr := h.buildReady(database, c.userID); readyErr == nil {
// Look up role for permission-filtered ready payload.
var userRole *db.Role
if role, rErr := database.GetRoleByID(c.user.RoleID); rErr == nil {
userRole = role
}
if ready, readyErr := h.buildReady(database, c.userID, userRole); readyErr == nil {
slog.Info("ws sending ready payload", "user_id", c.userID, "payload_bytes", len(ready))
if err := conn.Write(ctx, websocket.MessageText, ready); err != nil {
slog.Warn("ws: failed to send ready payload", "user_id", c.userID, "err", err)
@@ -332,7 +338,7 @@ func (h *Hub) buildAuthOK(user *db.User, roleName string) []byte {
// buildReady constructs the ready server→client message.
// Per PROTOCOL.md, channels include unread_count and last_message_id per user,
// and only protocol-specified fields (no slow_mode, archived, voice_* extras).
func (h *Hub) buildReady(database *db.DB, userID int64) ([]byte, error) {
func (h *Hub) buildReady(database *db.DB, userID int64, role *db.Role) ([]byte, error) {
channels, err := database.ListChannels()
if err != nil {
return nil, fmt.Errorf("buildReady ListChannels: %w", err)
@@ -348,6 +354,32 @@ func (h *Hub) buildReady(database *db.DB, userID int64) ([]byte, error) {
members = []db.MemberSummary{}
}
// Filter channels by READ_MESSAGES permission (mirrors REST handleListChannels).
overrides := map[int64]db.ChannelOverride{}
if role != nil && !permissions.HasAdmin(role.Permissions) {
var oErr error
overrides, oErr = database.GetAllChannelPermissionsForRole(role.ID)
if oErr != nil {
return nil, fmt.Errorf("buildReady GetAllChannelPermissionsForRole: %w", oErr)
}
}
var visibleChannels []db.Channel
for _, ch := range channels {
// When role is unavailable, include all channels (backwards compat).
if role == nil || permissions.HasAdmin(role.Permissions) {
visibleChannels = append(visibleChannels, ch)
continue
}
o := overrides[ch.ID]
effective := permissions.EffectivePerms(role.Permissions, o.Allow, o.Deny)
if effective&permissions.ReadMessages == permissions.ReadMessages {
visibleChannels = append(visibleChannels, ch)
}
}
if visibleChannels == nil {
visibleChannels = []db.Channel{}
}
// Per-user unread counts.
unreadMap, err := database.GetChannelUnreadCounts(userID)
if err != nil {
@@ -356,8 +388,8 @@ func (h *Hub) buildReady(database *db.DB, userID int64) ([]byte, error) {
}
// Build protocol-compliant channel objects (strip extra fields).
channelPayloads := make([]map[string]any, 0, len(channels))
for _, ch := range channels {
channelPayloads := make([]map[string]any, 0, len(visibleChannels))
for _, ch := range visibleChannels {
entry := map[string]any{
"id": ch.ID,
"name": ch.Name,
+15 -14
View File
@@ -3,10 +3,12 @@ package ws
import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"time"
"github.com/owncord/server/db"
"github.com/owncord/server/permissions"
)
@@ -74,28 +76,27 @@ func (h *Hub) handleVoiceJoin(ctx context.Context, c *Client, payload json.RawMe
h.handleVoiceLeave(ctx, c)
}
// Check channel capacity.
// Check channel capacity and persist to DB atomically.
maxUsers := ch.VoiceMaxUsers
if maxUsers > 0 {
existing, qErr := h.db.GetChannelVoiceStates(channelID)
if qErr != nil {
slog.Error("ws handleVoiceJoin GetChannelVoiceStates", "err", qErr, "channel_id", channelID)
c.sendMsg(buildErrorMsg(ErrCodeInternal, "failed to check channel capacity"))
if err := h.db.JoinVoiceChannelIfCapacity(c.userID, channelID, maxUsers); err != nil {
if errors.Is(err, db.ErrChannelFull) {
c.sendMsg(buildErrorMsg(ErrCodeChannelFull, "voice channel is full"))
return
}
slog.Error("ws handleVoiceJoin JoinVoiceChannelIfCapacity", "err", err, "user_id", c.userID)
c.sendMsg(buildErrorMsg(ErrCodeInternal, "failed to join voice channel"))
return
}
if len(existing) >= maxUsers {
c.sendMsg(buildErrorMsg(ErrCodeChannelFull, "voice channel is full"))
} else {
// No capacity limit — use standard join.
if err := h.db.JoinVoiceChannel(c.userID, channelID); err != nil {
slog.Error("ws handleVoiceJoin JoinVoiceChannel", "err", err, "user_id", c.userID)
c.sendMsg(buildErrorMsg(ErrCodeInternal, "failed to join voice channel"))
return
}
}
// Persist to DB.
if err := h.db.JoinVoiceChannel(c.userID, channelID); err != nil {
slog.Error("ws handleVoiceJoin JoinVoiceChannel", "err", err, "user_id", c.userID)
c.sendMsg(buildErrorMsg(ErrCodeInternal, "failed to join voice channel"))
return
}
// Load the persisted row immediately so later cleanup can target this exact
// join instance even if the user rejoins the same channel.
state, err := h.db.GetVoiceState(c.userID)