GHSA-2fp4-5v5c-4448 gonic: Path Traversal in playlist `id` bypasses ownership check, enabling any user to read/delete other users' playlists
SummaryThe maintainer's recent fix in [`6dd71e6a3c966867ef8c900d359a7df75789f410`](https://github.com/sentriz/gonic/commit/6dd71e6) (`fix(subsonic): enforce playlist ownership on getPlaylist/deletePlaylist`) added an ownership check based on `playlist.UserID`. However, `playlist.UserID` is derived from the *first path segment* of the attacker-controlled playlist ID, with no path containment on the resolved file path.**Any authenticated Subsonic user** can therefore bypass the ownership check and:**Read any other user's playlist** (name, comment, IsPublic flag, song list) by crafting a base64-encoded playlist ID whose first segment matches their own user ID, followed by `..` traversal segments pointing into another user's playlist directory.**Delete any other user's playlist** (including admin's curated playlists) by the same trick against `deletePlaylist`.**Probe arbitrary file paths on the host** for existence/readability.This is a bypass of the boundary the 6dd71e6 fix is trying to enforce; it is closely related to the original GONIC-1 IDOR but uses a different primitive (path traversal in the `id` parameter rather than direct cross-user access).Root cause`server/ctrlsubsonic/handlers_playlist.go::playlistIDDecode` performs raw base64 decode of the `id` parameter and passes the byte string straight to `playlistStore.Read/Delete`:`playlist/playlist.go::Store.Read` then:`userIDFromPath` reads only the first segment via `firstPathEl(relPath)` (`strconv.Atoi` of `strings.Split(path, "/")[0]`). It does not validate that the cleaned absolute path stays under `s.basePath`.The `id` parameter is base64-decoded as raw bytes (no path cleaning at decode time), so a payload like `"2/../../<victim>/playlist.m3u"` is preserved verbatim. `userIDFromPath` extracts `"2"` (the attacker's own user ID), `playlist.UserID = 2`, and the ownership check `playlist.UserID != user.ID && !playlist.IsPublic` becomes `2 != 2 && ...` → **false** → access allowed. Meanwhile `filepath.Join` resolves the `..` segments and escapes `basePath`.Affected code`playlist/playlist.go:88-144` — `Store.Read` joins `relPath` with `basePath` without containment validation`playlist/playlist.go:200-206` — `Store.Delete` (same pattern)`playlist/playlist.go:208-220` — `userIDFromPath` / `firstPathEl` trust only the first path segment`server/ctrlsubsonic/handlers_playlist.go:51-72` — `ServeGetPlaylist` ownership check`server/ctrlsubsonic/handlers_playlist.go:182-202` — `ServeDeletePlaylist` ownership check`server/ctrlsubsonic/handlers_playlist.go:209-212` — `playlistIDDecode` (no validation)Live PoC — passing Go testDrop this into `server/ctrlsubsonic/handlers_playlist_read_traversal_test.go` and run `go test -run TestGetPlaylistArbitraryRead_NonAdmin_UserIDPrefix ./server/ctrlsubsonic/ -v`:Test output against current `master` HEAD `6dd71e6`:The same approach against `ServeDeletePlaylist` (`f.contr.ServeDeletePlaylist`) deletes the targeted file.HTTP-level reproductionImpact**Confidentiality**: Any authenticated user can read any other user's playlist content, including the private (`IsPublic=false`) playlists that the recent 6dd71e6 fix specifically tried to protect.**Integrity / Availability**: Any authenticated user can delete any other user's playlists, including admin's curated lists. Same bypass technique works against `ServeDeletePlaylist`.**Trust boundary**: gonic explicitly supports multi-user deployments. This bug defeats the user-to-user authorization model that the maintainer just patched.**Arbitrary file content read** is *constrained* by gonic's M3U parser — only `#GONIC-NAME:` / `#GONIC-COMMENT:` attributes from the target file survive parsing. File-existence probing works against arbitrary paths.CVSS`CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:N` = **7.1 High**Suggested fixAdd path containment in `playlist/playlist.go` for `Store.Read`, `Store.Write`, and `Store.Delete` — reject any `relPath` that escapes `s.basePath` after `filepath.Join`:Apply in `Write()` (line 153) and `Delete()` (line 206) as well. The ownership check at 6dd71e6 then becomes a defense-in-depth layer on top of the structural containment.CreditsReported by Vishal Shukla ([@shukla304](https://github.com/shukla304) / [@therawdev](https://github.com/therawdev)).
CVSS v3.1 score: 7.1