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
117 stars 15 forks source link

Fix out-of-bounds read bug in Scrobbler.cxx:as_md5 due to missing null-termination #48

Closed bryal closed 1 year ago

bryal commented 1 year ago

buffer is not null terminated, but is implicitly interpreted as a C-string in the call to md5_hex. The fix is to convert it to a std::string beforehand, with its length (MD5_HEX_SIZE) given explicitly.

Fixes #47

MaxKellermann commented 1 year ago

That's not a good solution because it always allocates memory, and that's not necessary. Can you go without? e.g. by using std::string_view?

bryal commented 1 year ago

Good suggestion! I will apply this presently.

bryal commented 1 year ago

Is this better? There is no overload of + for string_view, so I had to construct the intermediary std::string explicitly, which was previously created transiently by the password_md5 + timestamp expression. At least there's only a single allocation again now.

MaxKellermann commented 1 year ago

Yes, better. And I wish we could eliminate that allocation by doing incremental MD5. I'll check that.