From 73b5d0a9b9ace89edcee30d9d83eb16c27c6f9b2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 09:27:53 +0100 Subject: [PATCH 01/13] system/Error: truncate the snprintf() return value snprintf() does not return the (truncated) length actually written, but the length that would be needed if the buffer were large enough. This API usage mistake in FormatLastError() can lead to overflow of the stack buffer, crashing the process (Windows only). Closes https://github.com/MusicPlayerDaemon/MPD/issues/1676 --- NEWS | 2 ++ src/system/Error.hxx | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d3e2ed88a..44183827a 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ ver 0.23.11 (not yet released) * macOS: fix build failure "no archive members specified" +* Windows + - fix crash bug (stack buffer overflow) after I/O errors * Android/Windows - update OpenSSL to 3.0.7 diff --git a/src/system/Error.hxx b/src/system/Error.hxx index aa017966b..b6b5c1fc1 100644 --- a/src/system/Error.hxx +++ b/src/system/Error.hxx @@ -70,8 +70,11 @@ FormatLastError(DWORD code, const char *fmt, Args&&... args) noexcept { char buffer[512]; const auto end = buffer + sizeof(buffer); - size_t length = snprintf(buffer, sizeof(buffer) - 128, + constexpr std::size_t max_prefix = sizeof(buffer) - 128; + size_t length = snprintf(buffer, max_prefix, fmt, std::forward(args)...); + if (length >= max_prefix) + length = max_prefix - 1; char *p = buffer + length; *p++ = ':'; *p++ = ' '; From eaecbcafb296ac7426064d7098d0febc2c86f8c9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 09:48:57 +0100 Subject: [PATCH 02/13] PlaylistFile: disallow backslash in playlist names on Windows The function spl_valid_name() should verify playlist names and prevent path traversal, but it failed to do so on Windows, because it forgot to check for backslashes. This buggy piece of code was already present when stored playlists were initially implemented in 2006 by commit 08003904d7af58c04, and even during the many rounds of code refactoring, nobody ever bothered to verify it. D'oh! (Thanks, Paul Arzelier) --- NEWS | 1 + src/PlaylistFile.cxx | 3 +++ 2 files changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 44183827a..1eec3f592 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,7 @@ ver 0.23.11 (not yet released) * macOS: fix build failure "no archive members specified" * Windows - fix crash bug (stack buffer overflow) after I/O errors + - fix path traversal bug because backslash was allowed in playlist names * Android/Windows - update OpenSSL to 3.0.7 diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx index 7035136df..24cde1a04 100644 --- a/src/PlaylistFile.cxx +++ b/src/PlaylistFile.cxx @@ -81,6 +81,9 @@ spl_valid_name(const char *name_utf8) */ return std::strchr(name_utf8, '/') == nullptr && +#ifdef _WIN32 + std::strchr(name_utf8, '\\') == nullptr && +#endif std::strchr(name_utf8, '\n') == nullptr && std::strchr(name_utf8, '\r') == nullptr; } From d9eec8a4558436ee428465a0175fd744f64511ee Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 10:44:38 +0100 Subject: [PATCH 03/13] fs/StandardDirectory: do not use $RUNTIME_DIRECTORY on Android This is systemd specific, and Android doesn't have systemd. --- src/fs/StandardDirectory.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs/StandardDirectory.cxx b/src/fs/StandardDirectory.cxx index 2767d2c49..af86b6f13 100644 --- a/src/fs/StandardDirectory.cxx +++ b/src/fs/StandardDirectory.cxx @@ -297,7 +297,7 @@ GetUserRuntimeDir() noexcept AllocatedPath GetAppRuntimeDir() noexcept { -#ifdef __linux__ +#if defined(__linux__) && !defined(ANDROID) /* systemd specific; see systemd.exec(5) */ if (const char *runtime_directory = getenv("RUNTIME_DIRECTORY")) if (auto dir = StringView{runtime_directory}.Split(':').first; From 94f06f0946bd9d49d03ba0da057f2c28a2c8b05e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 14:03:43 +0100 Subject: [PATCH 04/13] fs/StandardDirectory: use mode=0777 in mkdir() call Of course, mode=0700 is more secure, but allowing other users access to new directories is a choice the user should make via umask(). If the user-chosen umask allows everybody access, MPD should probably respect that. --- src/fs/StandardDirectory.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs/StandardDirectory.cxx b/src/fs/StandardDirectory.cxx index af86b6f13..761ffa0d5 100644 --- a/src/fs/StandardDirectory.cxx +++ b/src/fs/StandardDirectory.cxx @@ -308,7 +308,7 @@ GetAppRuntimeDir() noexcept #ifdef USE_XDG if (const auto user_dir = GetUserRuntimeDir(); !user_dir.IsNull()) { auto dir = user_dir / Path::FromFS("mpd"); - mkdir(dir.c_str(), 0700); + mkdir(dir.c_str(), 0777); return dir; } #endif From 1da974e3fa7344d351db399b1cc2ab8708bc788a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 10:32:45 +0100 Subject: [PATCH 05/13] fs/StandardDirectory: use PACKAGE_NAME from version.h --- src/fs/StandardDirectory.cxx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/fs/StandardDirectory.cxx b/src/fs/StandardDirectory.cxx index 761ffa0d5..ec2e3af04 100644 --- a/src/fs/StandardDirectory.cxx +++ b/src/fs/StandardDirectory.cxx @@ -53,6 +53,12 @@ #include "Main.hxx" #endif +#ifdef USE_XDG +#include "Version.h" // for PACKAGE_NAME +#define APP_FILENAME PATH_LITERAL(PACKAGE_NAME) +static constexpr Path app_filename = Path::FromFS(APP_FILENAME); +#endif + #if !defined(_WIN32) && !defined(ANDROID) class PasswdEntry { @@ -307,7 +313,7 @@ GetAppRuntimeDir() noexcept #ifdef USE_XDG if (const auto user_dir = GetUserRuntimeDir(); !user_dir.IsNull()) { - auto dir = user_dir / Path::FromFS("mpd"); + auto dir = user_dir / app_filename; mkdir(dir.c_str(), 0777); return dir; } From 4ded1ae67b6dd49c7f6e3f21e7f3b5a308d38111 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 14:13:10 +0100 Subject: [PATCH 06/13] fs/FileSystem: add CreateDirectoryNoThrow() --- src/fs/FileSystem.hxx | 10 ++++++++++ src/fs/StandardDirectory.cxx | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/fs/FileSystem.hxx b/src/fs/FileSystem.hxx index 0a742df9c..ebe80004f 100644 --- a/src/fs/FileSystem.hxx +++ b/src/fs/FileSystem.hxx @@ -67,6 +67,16 @@ StatFile(Path file, struct stat &buf, bool follow_symlinks = true) #endif +static inline bool +CreateDirectoryNoThrow(Path path) noexcept +{ +#ifdef _WIN32 + return CreateDirectory(path.c_str(), nullptr); +#else + return mkdir(path.c_str(), 0777); +#endif +} + /** * Truncate a file that exists already. Throws std::system_error on * error. diff --git a/src/fs/StandardDirectory.cxx b/src/fs/StandardDirectory.cxx index ec2e3af04..2a31d170b 100644 --- a/src/fs/StandardDirectory.cxx +++ b/src/fs/StandardDirectory.cxx @@ -314,7 +314,7 @@ GetAppRuntimeDir() noexcept #ifdef USE_XDG if (const auto user_dir = GetUserRuntimeDir(); !user_dir.IsNull()) { auto dir = user_dir / app_filename; - mkdir(dir.c_str(), 0777); + CreateDirectoryNoThrow(dir); return dir; } #endif From 06514aec6314749d05188cf31a9d40f014cf5976 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 10:39:28 +0100 Subject: [PATCH 07/13] fs/StandardDirectory: add GetAppCacheDir() --- src/fs/StandardDirectory.cxx | 18 ++++++++++++++++++ src/fs/StandardDirectory.hxx | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/src/fs/StandardDirectory.cxx b/src/fs/StandardDirectory.cxx index 2a31d170b..ad0ae2118 100644 --- a/src/fs/StandardDirectory.cxx +++ b/src/fs/StandardDirectory.cxx @@ -290,6 +290,24 @@ GetUserCacheDir() noexcept #endif } +AllocatedPath +GetAppCacheDir() noexcept +{ +#ifdef USE_XDG + if (const auto user_dir = GetUserCacheDir(); !user_dir.IsNull()) { + auto dir = user_dir / app_filename; + CreateDirectoryNoThrow(dir); + return dir; + } + + return nullptr; +#elif defined(ANDROID) + return context->GetCacheDir(Java::GetEnv()); +#else + return nullptr; +#endif +} + AllocatedPath GetUserRuntimeDir() noexcept { diff --git a/src/fs/StandardDirectory.hxx b/src/fs/StandardDirectory.hxx index 99cf0dc70..29746bc3a 100644 --- a/src/fs/StandardDirectory.hxx +++ b/src/fs/StandardDirectory.hxx @@ -43,6 +43,13 @@ GetUserMusicDir() noexcept; AllocatedPath GetUserCacheDir() noexcept; +/** + * Obtains cache directory for this application. + */ +[[gnu::const]] +AllocatedPath +GetAppCacheDir() noexcept; + /** * Obtains the runtime directory for the current user. */ From cfd4d5b13e1202d09ad4b0ac9f09714400d9ecd9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 13:54:03 +0100 Subject: [PATCH 08/13] StateFileConfig: use GetAppCacheDir() instead of GetUserCacheDir() --- src/StateFileConfig.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/StateFileConfig.cxx b/src/StateFileConfig.cxx index 453a657a4..a742e627b 100644 --- a/src/StateFileConfig.cxx +++ b/src/StateFileConfig.cxx @@ -32,7 +32,7 @@ StateFileConfig::StateFileConfig(const ConfigData &config) { #ifdef ANDROID if (path.IsNull()) { - const auto cache_dir = GetUserCacheDir(); + const auto cache_dir = GetAppCacheDir(); if (cache_dir.IsNull()) return; From 5d2e80f188038a6cd388d8ec6647a809243153a4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 13:54:40 +0100 Subject: [PATCH 09/13] db/Configured: use GetAppCacheDir() instead of GetUserCacheDir() --- NEWS | 2 ++ src/db/Configured.cxx | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 1eec3f592..5c2c091da 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.23.11 (not yet released) +* database + - simple: move default database to ~/.cache/mpd/db from ~/.cache/mpd.db * macOS: fix build failure "no archive members specified" * Windows - fix crash bug (stack buffer overflow) after I/O errors diff --git a/src/db/Configured.cxx b/src/db/Configured.cxx index 0ae9f0d11..fc4e2c328 100644 --- a/src/db/Configured.cxx +++ b/src/db/Configured.cxx @@ -51,11 +51,11 @@ CreateConfiguredDatabase(const ConfigData &config, } else { /* if there is no override, use the cache directory */ - const AllocatedPath cache_dir = GetUserCacheDir(); + const AllocatedPath cache_dir = GetAppCacheDir(); if (cache_dir.IsNull()) return nullptr; - const auto db_file = cache_dir / Path::FromFS(PATH_LITERAL("mpd.db")); + const auto db_file = cache_dir / Path::FromFS(PATH_LITERAL("db")); auto db_file_utf8 = db_file.ToUTF8(); if (db_file_utf8.empty()) return nullptr; From e9f6a3482c2a661ddd76122a458a78eda72f6cc1 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 13:58:37 +0100 Subject: [PATCH 10/13] db/Configured: add default "cache_directory" setting --- NEWS | 1 + src/db/Configured.cxx | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/NEWS b/NEWS index 5c2c091da..9dbe2ce4c 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.23.11 (not yet released) * database - simple: move default database to ~/.cache/mpd/db from ~/.cache/mpd.db + - simple: default "cache_directory" to ~/.cache/mpd/mounts * macOS: fix build failure "no archive members specified" * Windows - fix crash bug (stack buffer overflow) after I/O errors diff --git a/src/db/Configured.cxx b/src/db/Configured.cxx index fc4e2c328..beaad4bc4 100644 --- a/src/db/Configured.cxx +++ b/src/db/Configured.cxx @@ -24,6 +24,7 @@ #include "config/Param.hxx" #include "config/Block.hxx" #include "fs/AllocatedPath.hxx" +#include "fs/FileSystem.hxx" #include "fs/StandardDirectory.hxx" #include "util/RuntimeError.hxx" @@ -62,6 +63,19 @@ CreateConfiguredDatabase(const ConfigData &config, ConfigBlock block; block.AddBlockParam("path", std::move(db_file_utf8), -1); + + { + const auto mounts_dir = cache_dir + / Path::FromFS(PATH_LITERAL("mounts")); + CreateDirectoryNoThrow(mounts_dir); + + if (auto mounts_dir_utf8 = mounts_dir.ToUTF8(); + !mounts_dir_utf8.empty()) + block.AddBlockParam("cache_directory", + std::move(mounts_dir_utf8), + -1); + } + return DatabaseGlobalInit(main_event_loop, io_event_loop, listener, block); } From cac88e8be58ff5a88eb00a4f42fcfb5023793062 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 16:08:18 +0100 Subject: [PATCH 11/13] python/build/libs.py: re-enable verbose error strings This compile-time option is not about debug logging, but about curl_easy_strerror(). Closes https://github.com/MusicPlayerDaemon/MPD/issues/1670 --- NEWS | 1 + python/build/libs.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 9dbe2ce4c..1a24a3436 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,7 @@ ver 0.23.11 (not yet released) - fix path traversal bug because backslash was allowed in playlist names * Android/Windows - update OpenSSL to 3.0.7 + - re-enable CURL's verbose error strings ver 0.23.10 (2022/10/14) * storage diff --git a/python/build/libs.py b/python/build/libs.py index f50e164c6..4f35588e8 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -399,7 +399,6 @@ curl = CmakeProject( [ '-DBUILD_CURL_EXE=OFF', '-DBUILD_SHARED_LIBS=OFF', - '-DCURL_DISABLE_VERBOSE_STRINGS=ON', '-DCURL_DISABLE_LDAP=ON', '-DCURL_DISABLE_TELNET=ON', '-DCURL_DISABLE_DICT=ON', From a8b0c558188e7095f1ccc7a14705596335975b8d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 15:33:10 +0100 Subject: [PATCH 12/13] input/curl: make proxy verify setting optional These settings do not work if CURL was compiled with CURL_DISABLE_PROXY, and cause error "An unknown option was passed in to libcurl". Fixes regression by commit 7ab0dfc8ce56dacee9206b4c429a6c4bd94394b4 --- src/input/plugins/CurlInputPlugin.cxx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/input/plugins/CurlInputPlugin.cxx b/src/input/plugins/CurlInputPlugin.cxx index 864ea57ff..e90ce8daf 100644 --- a/src/input/plugins/CurlInputPlugin.cxx +++ b/src/input/plugins/CurlInputPlugin.cxx @@ -439,8 +439,14 @@ CurlInputStream::InitEasy() request->SetVerifyPeer(verify_peer); request->SetVerifyHost(verify_host); request->SetOption(CURLOPT_HTTPHEADER, request_headers.Get()); - request->SetProxyVerifyPeer(verify_peer); - request->SetProxyVerifyHost(verify_host); + + try { + request->SetProxyVerifyPeer(verify_peer); + request->SetProxyVerifyHost(verify_host); + } catch (...) { + /* these methods fail if libCURL was compiled with + CURL_DISABLE_PROXY; ignore silently */ + } } void From 9866adff957f9beefd4a3f3fda26281984e0cb2f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 28 Nov 2022 16:55:46 +0100 Subject: [PATCH 13/13] release v0.23.11 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 1a24a3436..7350eb4f5 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.23.11 (not yet released) +ver 0.23.11 (2022/11/28) * database - simple: move default database to ~/.cache/mpd/db from ~/.cache/mpd.db - simple: default "cache_directory" to ~/.cache/mpd/mounts