search

MusicPlayerDaemon / mpdscribble

a MPD client which submits information about tracks being played to a scrobbler (e.g. last.fm)
GNU General Public License v2.0
114 stars 15 forks source link

handshake with scrobble server fails if starting mpdscribble while music is playing #47

Closed bryal closed 1 year ago

bryal commented 1 year ago

If the music is paused in my MPD client, and I start mpdscribble, the scrobble server is successfully connected to.

$ mpdscribble -D 
2022-10-28T16:09:54+0200 starting mpdscribble (mdc 0.25)
2022-10-28T16:09:54+0200 loaded 0 songs from /var/cache/mpdscribble/lastfm.journal
2022-10-28T16:09:54+0200 connected to mpd 0.23.5 at localhost
2022-10-28T16:09:56+0200 [last.fm] handshake successful
2022-10-28T16:09:56+0200 [last.fm] session: be4d1e8a663229520bdc32688c53132f
2022-10-28T16:09:56+0200 [last.fm] now playing url: http://post.audioscrobbler.com:80/np_1.2
2022-10-28T16:09:56+0200 [last.fm] submit url: http://post2.audioscrobbler.com:80/protocol_1.2

If I instead try to start mpdscribble after having pressed the play button in my client, while music is playing, mpdscribble fails to handshake.

$ mpdscribble -D
2022-10-28T16:12:20+0200 starting mpdscribble (mdc 0.25)
2022-10-28T16:12:20+0200 loaded 0 songs from /var/cache/mpdscribble/lastfm.journal
2022-10-28T16:12:20+0200 connected to mpd 0.23.5 at localhost
2022-10-28T16:12:20+0200 new song detected (Kasatka - Debut), id: 4427, pos: 18

2022-10-28T16:12:22+0200 [last.fm] handshake failed, username or password incorrect (BADAUTH)
2022-10-28T16:12:22+0200 [last.fm] waiting 60 seconds before trying again

This reproduces every time I try it.

My config:

verbose = 0

[last.fm]
url = https://post.audioscrobbler.com/
username =...
password =...
journal = /var/cache/mpdscribble/lastfm.journal

The issue occurs in mpdscribble version 0.25. My system is Arch Linux. Installed via the AUR package mpdscribble-git.

$ mpdscribble --version
mpdscribble version 0.25
another audioscrobbler plugin for music player daemon.
Copyright 2005,2006 Kuno Woudt <kuno@frob.nl>.
Copyright 2008-2019 Max Kellermann <max.kellermann+mpdscribble@gmail.com>

mpdscribble comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of mpdscribble under the terms of the
GNU General Public License; either version 2 of the License, or
(at your option) any later version.
For more information about these matters, see the file named COPYING.

$ uname -a
Linux ... 5.19.11-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 24 Sep 2022 18:24:15 +0000 x86_64 GNU/Linux
bryal commented 1 year ago

I've located the source of the issue. It's here: https://github.com/MusicPlayerDaemon/mpdscribble/blob/031e023273affe7b3a1c61a253befb886a50eb39/src/Scrobbler.cxx#L351-L354

In as_md5, called by Scrobbler::Handshake, a char* is extracted from the resulting std:array of calling md5_hex. 

buffer = md5_hex(password);
password_md5 = buffer.data();

This char* is then implicitly interpreted as a string as part of the following call to md5_hex.

return md5_hex(password_md5 + timestamp);

The bug stems from the fact that password_md5 is not null terminated. Unless the memory following the end of password_md5 just happens to be 0 at runtime, out of bound reads will occur, and the returned hash will be incorrect. For me, this occured every time I tried to handshake while MPD was detected to be playing music.

I will publish a PR with a proposed fix for this shortly.