From fac56abb1d02fe5d7e8131d84c0f39d329fb33c7 Mon Sep 17 00:00:00 2001 From: bobiqqq Date: Wed, 26 Aug 2026 11:40:56 +0300 Subject: [PATCH] =?UTF-8?q?tiktak:=20LFI/auth-bypass=20=D0=B2=20/vtt/,=20?= =?UTF-8?q?=D1=83=D1=82=D0=B5=D1=87=D0=BA=D0=B0=20=D0=BF=D1=80=D0=B5=D0=B2?= =?UTF-8?q?=D1=8C=D1=8E,=20=D0=BA=D1=80=D0=B5=D0=B4=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handleVtt: id валидируется как положительное целое, ошибка GetVideo больше не игнорируется. Закрывает сразу два вектора: обход ACL через нулевой db.Video (Private=false => haveAccess пропускал анонима) и path traversal в path.Join(VttFolder, vid+".vtt"). - handleCreate: резкое превью приватного видео удаляется после блюра, иначе оно оставалось доступным в public/static через echo.Static. - main.go: DSN приведён к паролю из compose (5935004 поменял только compose, из-за чего сервис не достучался бы до базы). - db: пароли хешируются sha256 с pepper, сверка в Go constant-time, плейнтекст принимается как legacy => старые юзеры не теряют доступ. - cookies: HttpOnly + Path=/ + SameSite=Lax. Co-Authored-By: Claude Opus 5 --- services/tiktak/db/db.go | 35 ++++++++++++++++++++++++-- services/tiktak/docker-compose.yml | 6 +++++ services/tiktak/main.go | 5 +++- services/tiktak/server/auth/context.go | 14 ++++++++++- services/tiktak/server/server.go | 30 ++++++++++++++++++---- 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/services/tiktak/db/db.go b/services/tiktak/db/db.go index 10c83f0..ddcb312 100644 --- a/services/tiktak/db/db.go +++ b/services/tiktak/db/db.go @@ -2,9 +2,35 @@ package db import ( "context" + "crypto/sha256" + "crypto/subtle" + "fmt" + "github.com/gocraft/dbr/v2" ) +// pepper is mixed into every stored password. NEVER change it after the first +// deploy -- every account created afterwards would be locked out. +const pepper = "cRazy_P4ssw0rd_MeOw_Me0W_676767676767" + +// HashPassword returns the value that is actually persisted in users.password. +func HashPassword(password string) string { + h := sha256.New() + h.Write([]byte(pepper)) + h.Write([]byte(password)) + return fmt.Sprintf("sha256$%x", h.Sum(nil)) +} + +// passwordMatches accepts the new hashed form and, for accounts created before +// this patch, the legacy plaintext form -- otherwise existing users (including +// the checker's) would be locked out and the SLA would drop. +func passwordMatches(stored, supplied string) bool { + if subtle.ConstantTimeCompare([]byte(stored), []byte(HashPassword(supplied))) == 1 { + return true + } + return subtle.ConstantTimeCompare([]byte(stored), []byte(supplied)) == 1 +} + type Storage struct { conn *dbr.Connection } @@ -30,6 +56,7 @@ func (s *Storage) init() { func (s *Storage) InsertUser(ctx context.Context, user *User) error { sess := s.conn.NewSession(nil) + user.Password = HashPassword(user.Password) _, err := sess.InsertInto("users").Columns("login", "password").Record(user).ExecContext(ctx) return err } @@ -37,15 +64,19 @@ func (s *Storage) InsertUser(ctx context.Context, user *User) error { func (s *Storage) FindUser(ctx context.Context, login, password string) (*User, error) { sess := s.conn.NewSession(nil) user := new(User) + // Look the account up by login only, then verify the secret in Go: the + // password no longer travels through the WHERE clause as a comparable value. err := sess.Select("*"). From("users"). Where("login = ?", login). - Where("password = ?", password). LoadOneContext(ctx, user) if err != nil { return nil, err } - return user, err + if !passwordMatches(user.Password, password) { + return nil, dbr.ErrNotFound + } + return user, nil } func (s *Storage) AddVideo(ctx context.Context, v *Video) error { diff --git a/services/tiktak/docker-compose.yml b/services/tiktak/docker-compose.yml index 6eb6eeb..c62b8c8 100644 --- a/services/tiktak/docker-compose.yml +++ b/services/tiktak/docker-compose.yml @@ -2,6 +2,9 @@ version: '3' services: tiktak: build: . + # CI (.gitea/workflows/build-and-push.yml) pushes only images matching + # git.itqdev.xyz/4x10m/* -- without this line the build job fails. + image: git.itqdev.xyz/4x10m/tiktak:${IMAGE_TAG:-latest} ports: - "5000:4000" restart: always @@ -13,8 +16,11 @@ services: db: image: mysql:8.0.17 restart: always + # NOTE: not published to the host on purpose -- keep it that way. volumes: - ./db_data:/var/lib/mysql environment: + # Rotated. Keep in sync with the DSN in main.go. + # Only applied when ./db_data is created from scratch! MYSQL_ROOT_PASSWORD: cRazy_P4ssw0rd_MeOw_Me0W_676767676767 MYSQL_DATABASE: tiktak diff --git a/services/tiktak/main.go b/services/tiktak/main.go index 47ff6b6..50335c9 100644 --- a/services/tiktak/main.go +++ b/services/tiktak/main.go @@ -17,9 +17,12 @@ import ( ) func main() { + // Rotated DB password. Must stay in sync with MYSQL_ROOT_PASSWORD in + // docker-compose.yml AND with the actual MySQL user (ALTER USER) when the + // ./db_data volume already exists. conn, err := dbr.Open( "mysql", - "root:root@tcp(db:3306)/tiktak", nil) + "root:cRazy_P4ssw0rd_MeOw_Me0W_676767676767@tcp(db:3306)/tiktak", nil) if err != nil { panic(err) diff --git a/services/tiktak/server/auth/context.go b/services/tiktak/server/auth/context.go index 1ead9fe..6f83cb3 100644 --- a/services/tiktak/server/auth/context.go +++ b/services/tiktak/server/auth/context.go @@ -11,7 +11,19 @@ const IdCookieName = "user_id" func Cookies(id int64) (*http.Cookie, *http.Cookie) { ids := strconv.FormatInt(id, 10) - return &http.Cookie{Value: ids, Name: IdCookieName}, &http.Cookie{Value: Generate(ids), Name: HashCookieName} + return &http.Cookie{ + Name: IdCookieName, + Value: ids, + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }, &http.Cookie{ + Name: HashCookieName, + Value: Generate(ids), + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + } } type Context struct { diff --git a/services/tiktak/server/server.go b/services/tiktak/server/server.go index 592c907..ea818a3 100644 --- a/services/tiktak/server/server.go +++ b/services/tiktak/server/server.go @@ -136,6 +136,12 @@ func (s *Server) handleCreate(c echo.Context) error { if err = video.Blur(ppath, s.privatePreviewPath(v.ID)); err != nil { return c.Render(http.StatusUnprocessableEntity, "create", ErrorResponse{"failed to blur preview: " + err.Error()}) } + // public/static is served by echo.Static, so the sharp preview of a + // private video would otherwise be downloadable by anyone at + // /public/static/preview_.png -- drop it once it has been blurred. + if err = os.Remove(ppath); err != nil && !os.IsNotExist(err) { + return c.Render(http.StatusUnprocessableEntity, "create", ErrorResponse{"failed to drop preview: " + err.Error()}) + } } return c.Redirect(http.StatusFound, fmt.Sprintf("/watch/%d", v.ID)) } @@ -180,14 +186,28 @@ func (s *Server) handleWatch(c echo.Context) error { func (s *Server) handleVtt(c echo.Context) error { ac := c.(*auth.Context) - vid := c.QueryParam("id") - uid := ac.GetId() + + // The id must be a plain positive integer. Anything else (./7, x/../7, + // ../../etc/foo) is rejected before it can reach either the DB or the FS. + vid, err := strconv.ParseInt(c.QueryParam("id"), 10, 64) + if err != nil || vid <= 0 { + return c.NoContent(http.StatusNotFound) + } + ctx := context.Background() - v, _ := s.db.GetVideo(ctx, vid) - if !s.haveAccess(ctx, uid, *v) { + v, err := s.db.GetVideo(ctx, vid) + // The error MUST be handled: a missing row leaves a zero db.Video + // (UserID=0, Private=false) which haveAccess() would happily accept. + if err != nil || v == nil { + return c.NoContent(http.StatusNotFound) + } + + if !s.haveAccess(ctx, ac.GetId(), *v) { return c.NoContent(http.StatusForbidden) } - vttPath := path.Join(s.c.VttFolder, vid+".vtt") + + // Build the path from the validated integer, never from raw user input. + vttPath := path.Join(s.c.VttFolder, strconv.FormatInt(vid, 10)+".vtt") return c.File(vttPath) }