Skip to content

Fix path traversal in lyrics filename when storing lyrics in song dir - #658

Open
acts-1631 wants to merge 1 commit into
ncmpcpp:masterfrom
acts-1631:fix/path-traversal
Open

acts-1631 wants to merge 1 commit into
ncmpcpp:masterfrom
acts-1631:fix/path-traversal

Conversation

@acts-1631

Copy link
Copy Markdown

lyricsFilename joined the raw MPD song URI with mpd_music_dir in the
store_lyrics_in_song_dir branch without filtering '..' components or
absolute paths. Song::getURI returns mpd_song_get_uri verbatim and
Song::isStream only rejects http(s):// prefixes, so a non-stream URI
such as ../../home/user/.config/foo reached std::ofstream and wrote
fetched lyrics content to an attacker-chosen path outside the music
directory (with a forced .txt suffix).

Strip '.' and '..' components and leading separators from the URI
before joining so the resulting path stays confined under mpd_music_dir.
This mirrors the sibling lyrics_directory branch, which already calls
removeInvalidCharsFromFilename to strip '/'.

lyricsFilename joined the raw MPD song URI with mpd_music_dir in the
store_lyrics_in_song_dir branch without filtering '..' components or
absolute paths. Song::getURI returns mpd_song_get_uri verbatim and
Song::isStream only rejects http(s):// prefixes, so a non-stream URI
such as ../../home/user/.config/foo reached std::ofstream and wrote
fetched lyrics content to an attacker-chosen path outside the music
directory (with a forced .txt suffix).

Strip '.' and '..' components and leading separators from the URI
before joining so the resulting path stays confined under mpd_music_dir.
This mirrors the sibling lyrics_directory branch, which already calls
removeInvalidCharsFromFilename to strip '/'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant