From 7e65725253e40dce726d1be441b33b72d72ea8c4 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Wed, 20 May 2026 14:08:46 +0300 Subject: Escape LIKE wildcards in search and validate password on CreateUser (j9+i9) - Extract escapeLike helper in repository/media.go to explicitly name and encapsulate the LIKE wildcard sanitisation (%, _, \) used by ListMedia, preventing timing-based SQL injection via crafted search terms. - Add ErrWeakPassword sentinel and minPasswordLen=8 constant; reject blank or short passwords in userAdminService.CreateUser before hashing. - Map ErrWeakPassword to HTTP 400 in handleError. - Update service tests to use an 8-char password and add weak/empty cases. Co-Authored-By: Claude Opus 4.7 --- internal/api/handlers.go | 3 ++- internal/repository/media.go | 17 ++++++++++++----- internal/service/admin_test.go | 2 +- internal/service/service.go | 1 + internal/service/user.go | 8 ++++++++ internal/service/user_test.go | 24 +++++++++++++++++++----- 6 files changed, 43 insertions(+), 12 deletions(-) diff --git a/internal/api/handlers.go b/internal/api/handlers.go index b276b10..4c3517a 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -57,7 +57,8 @@ func handleError(w http.ResponseWriter, err error) { writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "invalid credentials"}) case errors.Is(err, service.ErrUnsupportedExtension), errors.Is(err, service.ErrInvalidFeed), - errors.Is(err, service.ErrCannotDeleteSelf): + errors.Is(err, service.ErrCannotDeleteSelf), + errors.Is(err, service.ErrWeakPassword): badRequest(w, err.Error()) default: writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) diff --git a/internal/repository/media.go b/internal/repository/media.go index a7a9553..c40b511 100644 --- a/internal/repository/media.go +++ b/internal/repository/media.go @@ -181,12 +181,9 @@ func (s *SQLite) ListMedia(ctx context.Context, filter MediaFilter) ([]model.Med query := `SELECT DISTINCT media.id, media.set_id, media.rel_path, media.file_name, media.abs_path, media.type, media.duration, media.codec, media.resolution, media.bitrate, media.file_size_bytes, media.width, media.height, media.exif_camera, media.exif_lens, media.exif_date, media.exif_iso, media.exif_f_number, media.exif_exposure, media.exif_focal_length, media.thumbnail_path, media.play_count, media.deleted_at, media.created_at FROM media` if filter.Search != "" { + // escapeLike escapes LIKE wildcards so user input is treated as a literal substring. conds = append(conds, `(media.file_name LIKE ? ESCAPE '\' OR media.rel_path LIKE ? ESCAPE '\')`) - term := filter.Search - term = strings.ReplaceAll(term, "\\", "\\\\") - term = strings.ReplaceAll(term, "%", "\\%") - term = strings.ReplaceAll(term, "_", "\\_") - like := "%" + term + "%" + like := "%" + escapeLike(filter.Search) + "%" args = append(args, like, like) } if filter.Favorites { @@ -307,6 +304,16 @@ func (s *SQLite) IncrementPlayCount(ctx context.Context, id int64) error { return nil } +// escapeLike escapes backslash, percent, and underscore in s so it can be +// used as a literal substring in a SQL LIKE ? ESCAPE '\' clause without +// allowing timing-based wildcard injection. +func escapeLike(s string) string { + s = strings.ReplaceAll(s, `\`, `\\`) + s = strings.ReplaceAll(s, `%`, `\%`) + s = strings.ReplaceAll(s, `_`, `\_`) + return s +} + func placeholders(n int) string { if n <= 0 { return "" diff --git a/internal/service/admin_test.go b/internal/service/admin_test.go index 0e4d309..1a93356 100644 --- a/internal/service/admin_test.go +++ b/internal/service/admin_test.go @@ -200,7 +200,7 @@ func TestAdminService_CreateUser(t *testing.T) { } hasher := &fakeHasher{fixed: "hashed", err: tt.hashErr} svc := NewAdminService(store, newMockClock(), hasher, nil, "", ctx) - user, err := svc.CreateUser(ctx, "alice", "secret", false) + user, err := svc.CreateUser(ctx, "alice", "strongpass", false) if tt.wantErr { if err == nil { t.Fatal("expected error") diff --git a/internal/service/service.go b/internal/service/service.go index ba826be..2481a6d 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -22,6 +22,7 @@ var ( ErrInvalidCredentials = errors.New("invalid credentials") ErrInvalidFeed = errors.New("invalid feed") ErrCannotDeleteSelf = errors.New("cannot delete self") + ErrWeakPassword = errors.New("password must be at least 8 characters") ) // MediaQueryFilter defines query parameters for listing media from the API layer. diff --git a/internal/service/user.go b/internal/service/user.go index 01467ed..ba3e541 100644 --- a/internal/service/user.go +++ b/internal/service/user.go @@ -10,6 +10,9 @@ import ( "codeberg.org/snonux/player/internal/repository" ) +// minPasswordLen is the minimum acceptable password length for new accounts. +const minPasswordLen = 8 + // userAdminService handles user account management. type userAdminService struct { store repository.UserAdminServiceStore @@ -27,6 +30,11 @@ func (s *userAdminService) ListUsers(ctx context.Context) ([]model.User, error) } func (s *userAdminService) CreateUser(ctx context.Context, username, password string, isAdmin bool) (*model.User, error) { + // Reject blank or short passwords before hashing to prevent weak account creation. + if len(password) < minPasswordLen { + return nil, ErrWeakPassword + } + hash, err := s.hasher.Hash(password) if err != nil { return nil, fmt.Errorf("hash password: %w", err) diff --git a/internal/service/user_test.go b/internal/service/user_test.go index 1e0ceb3..b470119 100644 --- a/internal/service/user_test.go +++ b/internal/service/user_test.go @@ -30,23 +30,37 @@ func TestUserAdminService_CreateUser(t *testing.T) { tests := []struct { name string + password string hashErr error createErr error wantErr bool }{ { - name: "ok", + name: "ok", + password: "strongpass", // 10 chars, meets 8-char minimum }, { - name: "hash error", - hashErr: errors.New("boom"), - wantErr: true, + name: "hash error", + password: "strongpass", + hashErr: errors.New("boom"), + wantErr: true, }, { name: "create error", + password: "strongpass", createErr: errors.New("boom"), wantErr: true, }, + { + name: "empty password rejected", + password: "", + wantErr: true, + }, + { + name: "short password rejected", + password: "short", + wantErr: true, + }, } for _, tt := range tests { @@ -60,7 +74,7 @@ func TestUserAdminService_CreateUser(t *testing.T) { } hasher := &fakeUserHasher{fixed: "hashed", err: tt.hashErr} svc := NewUserAdminService(store, clock.RealClock{}, hasher) - user, err := svc.CreateUser(ctx, "alice", "secret", false) + user, err := svc.CreateUser(ctx, "alice", tt.password, false) if tt.wantErr { if err == nil { t.Fatal("expected error") -- cgit v1.2.3